-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Install Prettier For Automatic Code Formatting (JS, HTML, & SCSS) #146
Conversation
✅ Deploy Preview for radiant-cucurucho-d09bae ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESLint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
// 1155 E 60th St | ||
'136212': {owner: BuildingOwners.uchicago.key}, | ||
"136212": { owner: BuildingOwners.uchicago.key }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
God bless automation cleaning up this giant file 😆
@@ -128,7 +110,9 @@ | |||
// A column is empty if it's all empty string or '0', so skip it if so. Some columns switch | |||
// between both, like Natural Gas Use on Merch Mart, which we also want to ignore | |||
return !this.historicBenchmarks.every((datum) => { | |||
return (datum as any)[colKey] === '' || (datum as any)[colKey] === '0.0'; | |||
return ( | |||
(datum as any)[colKey] === '' || (datum as any)[colKey] === '0.0' |
Check warning
Code scanning / ESLint
Disallow the `any` type Warning
Copilot Autofix AI about 20 hours ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
@@ -128,7 +110,9 @@ | |||
// A column is empty if it's all empty string or '0', so skip it if so. Some columns switch | |||
// between both, like Natural Gas Use on Merch Mart, which we also want to ignore | |||
return !this.historicBenchmarks.every((datum) => { | |||
return (datum as any)[colKey] === '' || (datum as any)[colKey] === '0.0'; | |||
return ( | |||
(datum as any)[colKey] === '' || (datum as any)[colKey] === '0.0' |
Check warning
Code scanning / ESLint
Disallow the `any` type Warning
Copilot Autofix AI about 20 hours ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
class="unit" | ||
v-html="' ' + unit" | ||
/> | ||
<span v-if="unit" class="unit" v-html="' ' + unit" /> |
Check warning
Code scanning / ESLint
disallow use of v-html to prevent XSS attack Warning
class="unit" | ||
v-html="unit" | ||
/> | ||
{{ statValue }} <span class="unit" v-html="unit" /> |
Check warning
Code scanning / ESLint
disallow use of v-html to prevent XSS attack Warning
class="label" | ||
v-html="graphTitle" | ||
/> | ||
<div class="label" v-html="graphTitle" /> |
Check warning
Code scanning / ESLint
disallow use of v-html to prevent XSS attack Warning
mapPopup: any, | ||
googleMapsSearchInput: any, | ||
mapPopup: any; | ||
googleMapsSearchInput: any; |
Check warning
Code scanning / ESLint
Disallow the `any` type Warning
Copilot Autofix AI about 20 hours ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
this.Leaflet.marker(coordinates, MarkerOptions) | ||
.addTo(this.mainFeatureGroup!); | ||
this.Leaflet.marker(coordinates, MarkerOptions).addTo( | ||
this.mainFeatureGroup!, |
Check warning
Code scanning / ESLint
Disallow non-null assertions using the `!` postfix operator Warning
Copilot Autofix AI about 20 hours ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
const SearchRadiusMeters = | ||
MapPage.OneMileInMeters * this.formSearchDistanceMiles; | ||
this.Leaflet.circle(coordinates, { radius: SearchRadiusMeters }).addTo( | ||
this.mainFeatureGroup!, |
Check warning
Code scanning / ESLint
Disallow non-null assertions using the `!` postfix operator Warning
Copilot Autofix AI about 20 hours ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
const marker = this.Leaflet.marker(buildingCoords, MarkerOptions) | ||
.addTo(this.mainFeatureGroup!); | ||
const marker = this.Leaflet.marker(buildingCoords, MarkerOptions).addTo( | ||
this.mainFeatureGroup!, |
Check warning
Code scanning / ESLint
Disallow non-null assertions using the `!` postfix operator Warning
Copilot Autofix AI about 20 hours ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
} as any); | ||
}); | ||
maxWidth: 'auto', | ||
} as any, |
Check warning
Code scanning / ESLint
Disallow the `any` type Warning
Copilot Autofix AI about 20 hours ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
@@ -1699,12 +1707,15 @@ export const BuildingsCustomInfo: { [buildingId: string]: IBuildingCustomInfo } | |||
// 162 North State Street | |||
'250167': { owner: BuildingOwners.saic.key }, | |||
// 280 Building, 280 S Columbus Dr | |||
'2tags: [ BuildingTags.hasRetrofitCaseStudy],52065': { owner: BuildingOwners.saic.key , | |||
'2tags: [ BuildingTags.hasRetrofitCaseStudy],52065': { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Fix this line, this looks derped
A happy coincidence, I didn't run linting in 8d40de9, and thus confirmed Prettier works in CI 🎉 Note that commit fixes a building (280 N Columbus) not being associated with its retrofit report or its owner. Here's after the fix: |
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me.
Description
Installed Prettier, following the docs. I ran it leaving the defaults on and everything seemed generally fine except quotes (the default is double quotes). This also applies to SCSS, so I think this should remove the need for Stylelint. Here's a simple demo:
I've also:
yarn lint
andyarn lint-fix
run both ESLint & then PrettierFixes #93
Testing Instructions
Prettier is pretty well tested, so just spot check the code and make sure the changes make sense, but mainly poke around the app and make sure everything still looks fine. To test locally, mess up some spacing, and run
yarn lint
to see errors oryarn lint-fix
to fix them!Additionally, I confirmed having pending Prettier changes fails CI - see commit 8d40de9 near the bottom, which I pushed up without running
yarn lint-fix
.Checklist: