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

format files in the root directory #15

Merged
merged 5 commits into from
May 22, 2020
Merged

format files in the root directory #15

merged 5 commits into from
May 22, 2020

Conversation

ryanatkn
Copy link
Owner

This adds an additional glob to the default formatter to include some files in the root directory. Which files to target is a bit tricky. Generally speaking, Gro expects everything to be in your src/ directory, but it's common to have some files in the root directory for various tools, and we want a subset of those to be formatted. We don't want to format things like package-lock.json or package.json because those are modified directly by npm. Insead of adding a blacklist, I opted to just target TypeScript, JavaScript, and Markdown files. Seems ok?

@greatbacon
Copy link
Contributor

Is it possible there might be other ts, js or md files someone other than us might keep in their root that they don't want formatted? Honestly a code cleaner targeting files outside the src directory feels like unexpected behavior to me. This seems like a potential sharp edge for users, especially considering it targets a different subset of files than it does in the src folder.

@ryanatkn
Copy link
Owner Author

ryanatkn commented May 22, 2020

Good point, I can see how that's surprising behavior. In the Node ecosystem it's common to have *.config.js (or ts - we have a rollup.config.js right now in Felt that Sapper uses) files in the root directory, along with the normal README, CHANGELOG, CONTRIBUTING, etc. I personally want those formatted and can't think of any I wouldn't, but it's weird that it omits all other file types. I'm unable to think of examples I wouldn't want formatted but the special casing makes it look wrong.

I noticed this before but just confirmed - prettier and npm are formatting package.json and package-lock.json identically. So at least for our projects with current behavior, we could use the same set of file extensions. If CI ever breaks because of npm not matching the formatting, we could re-evaluate. How does that sound? gro format works consistently on both the root directory (not nested dirs) and src/**/*?

@greatbacon
Copy link
Contributor

Alright cool, that works for me.

@ryanatkn ryanatkn changed the title format certain files in the root directory format files in the root directory May 22, 2020
@ryanatkn
Copy link
Owner Author

Great alright it now uses the same set of extensions.

@ryanatkn ryanatkn merged commit 324b33c into master May 22, 2020
@ryanatkn ryanatkn deleted the format-root-markdown branch May 22, 2020 19:08
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