-
Notifications
You must be signed in to change notification settings - Fork 715
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
Added Vale to site checks #4231
Conversation
To run vale:
Tested, runs fine.
A couple things I would like to see:
Concern:
|
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.
Hard to test everything, but this PR isn't forcing anyone to use it and we can improve it/pare down through our own regular use, so lgtm with that in mind
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.
Decided to actually fit it to the google style guide before merging
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.
Woah! @atsansone, before merging a new tool like this, we need to see it staged, try it out, and make a team decision. Have you staged this?
On further thought, I'm closing this PR for now. We need a lot more research before adding a tool like this. We should probably get @khanhnwin involved if we feel it's important enough. (To be honest, given my current understanding, I don't feel like I need this. Hence the research and testing.) |
Visit the preview URL for this PR (updated for commit be8f7e8): https://dart-dev--pr4231-vale-implementation-rsgx3g06.web.app |
fc4965d
to
be8f7e8
Compare
Moved to dart-lang/site-shared#223 per conversation with @parlough . |
No description provided.