Skip to content

Conversation

@sidvishnoi
Copy link
Member

@sidvishnoi sidvishnoi commented Mar 22, 2020

// Generated on Fri Feb 26 2016 13:09:51 GMT+1100 (AEDT)
"use strict";
module.exports = function(config) {
module.exports = function (config) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of these changes - a space after "function" keyword

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth to check the relevant discussion...

Copy link
Collaborator

@saschanaz saschanaz Mar 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prettier/prettier#3903 is the culprit (an old PR but was merged into next branch), and seemingly the majority of people liked it.

@sidvishnoi sidvishnoi requested a review from saschanaz March 22, 2020 09:01
? html`
<span class="example-title">: ${report.title}</span>
`
? html` <span class="example-title">: ${report.title}</span> `
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to check we want the space or not. I think generally we don't.

// Generated on Fri Feb 26 2016 13:09:51 GMT+1100 (AEDT)
"use strict";
module.exports = function(config) {
module.exports = function (config) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth to check the relevant discussion...

.toLowerCase()
.replace(/[()]/g, "")
.replace(/\s/g, "-");
let last = name.toLowerCase().replace(/[()]/g, "").replace(/\s/g, "-");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No needless newlines 🎉

? html` <span class="example-title">: ${report.title}</span> `
: "";
return html`
<div class="marker">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With prettier 2.0 this can be:

html`<div>
  ...
</div>`;

This will reduce one indentation level, what do you think @sidvishnoi?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me. 👍

Copy link
Collaborator

@saschanaz saschanaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can merge this, with code formats to be enhanced separately.

@sidvishnoi sidvishnoi merged commit 5a3c14d into develop Mar 22, 2020
@sidvishnoi sidvishnoi deleted the greenkeeper/prettier-2.0.1 branch March 22, 2020 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants