Skip to content
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

prettierignore -- ignore .gitkeep and yaml #3214

Merged
merged 3 commits into from
Oct 18, 2023
Merged

Conversation

bcolloran
Copy link
Contributor

couple tiny updates to prettierignore

@bcolloran
Copy link
Contributor Author

@ericpgreen2 quick one here for you

@ericpgreen2
Copy link
Contributor

What's the motivation for this? Isn't it helpful to have Prettier check YAML? ChatGPT thinks we shouldn't make this change 😅

@bcolloran
Copy link
Contributor Author

bcolloran commented Oct 12, 2023

Motivation is to reduce noise on prettier check. For example, i get the following

.eslintignore[error] No parser could be inferred for file: .eslintignore
dev-project/dashboards/.gitkeep[error] No parser could be inferred for file: dev-project/dashboards/.gitkeep
[warn] dev-project/dashboards/sales_data_sample_model_dashboard.yaml
[warn] dev-project/dashboards/Users_dashboard.yaml
dev-project/models/.gitkeep[error] No parser could be inferred for file: dev-project/models/.gitkeep
dev-project/sources/.gitkeep[error] No parser could be inferred for file: dev-project/sources/.gitkeep
[warn] dev-project/sources/AdBids.yaml
[warn] dev-project/sources/AdImpressions.yaml
[warn] dev-project/sources/sales_data_sample.yaml
[warn] dev-project/sources/steam.yaml
[warn] dev-project/sources/Users.yaml
netlify.toml[error] No parser could be inferred for file: netlify.toml
rill[error] No parser could be inferred for file: rill
[warn] web-local/playwright-report/index.html
[warn] Code style issues found in 8 files. Forgot to run Prettier?

ChatGPT is hallucinating about .gitkeep files at least -- prettier does not support those. But yeah we can keep the yaml checks if you want -- I updated this PR to ignore the dev-project/ folder and .eslintignore and toml, but to check yaml files.

Copy link
Contributor

@ericpgreen2 ericpgreen2 left a comment

Choose a reason for hiding this comment

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

Cool, this works. I wonder though – instead of ignoring dev-project, should we be better formatting the YAML files we create to adhere to prettier?

@bcolloran bcolloran merged commit bf20fc4 into main Oct 18, 2023
1 check passed
@bcolloran bcolloran deleted the prettierignore-updates branch October 18, 2023 00:42
@bcolloran
Copy link
Contributor Author

merged this for now, but happy to revisit this if you wish -- I'm really not attached to this solution if you want to try something else :-)

My understanding was that this particular folder is really intended for user scratchpad, and not something that we should necessarily be putting a lot of effort into formatting. That's not right, we can add automatic formatting for the yaml files, But we should have a discussion about whether we want to force prettier formatting on all users including those who may not know what it's doing or may have their own particular desires about how they want their files to be formatted. I wast mostly thinking that this is probably the fastest path to reducing the noise in the pretty raw output for the time being.

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.

2 participants