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

New rules to improve the PR review #1433

Open
6 tasks
molant opened this issue Oct 22, 2018 · 3 comments
Open
6 tasks

New rules to improve the PR review #1433

molant opened this issue Oct 22, 2018 · 3 comments
Assignees

Comments

@molant
Copy link
Member

molant commented Oct 22, 2018

The PR review can take a while, especially for new comers. We should try to automate this process as much as possible and for that I think we should create a few new rules or checks:

  • Verify that url is always URL or URLs in the comments (HTML, SVG, PNG, etc.)
  • Make sure there's an ending . in the context.report (and maybe debug and comments?)
  • Don't use abbreviations (at least in context.report)
  • Make sure we use inclusive language, no Just usage, etc. (maybe look into alex, markdown-spellcheck, etc.)
  • Code is surrounded by ' in comments.
  • Prefer destructuring in the parameters directly. E.g:
- const validate = async (fetchEnd: FetchEnd) => {
-     const { element, resource, response }: { element: IAsyncHTMLElement | null, resource: string, response: Response } = fetchEnd;
+ const validate = async ({ element, resource, response }: FetchEnd) => { 

And some others.

@webhintio/core I'm sure I'm forgetting more things we can catch up early. Can you help?

@alrra
Copy link
Contributor

alrra commented Oct 23, 2018

Verify that url is always URL or URLs in the comments

HTML, SVG, PNG...

Make sure there's an ending . in the context.report (and maybe debug and comments?)

Code is surrounded by '.

@molant
Copy link
Member Author

molant commented Oct 23, 2018

Added those to the list. Not sure how we will detect code in comments though 🤔 Maybe a list of keywords?

@alrra
Copy link
Contributor

alrra commented Oct 24, 2018

Maybe a list of keywords?

Or when words are camelCase, or have dots and the letter after the dot is lowercase (e.g.: context.something).

@molant molant added this to the 1811-1 milestone Oct 29, 2018
@molant molant self-assigned this Oct 29, 2018
@antross antross modified the milestones: 1811-1, 1811-2 Nov 13, 2018
@molant molant removed this from the 1811-2 milestone Nov 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants