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

CI Docs: Using an inclusive linter #726

Merged
merged 7 commits into from
Oct 20, 2021
Merged

Conversation

jeremydelacruz
Copy link
Contributor

@jeremydelacruz jeremydelacruz commented Oct 18, 2021

Pull Request Template

What are you trying to address

Description of new changes

  • Adding new markdown file in docs/continuous-integration/inclusive-linting.md
  • Adding content around what inclusive linting is, how it can be useful, and how to get started with an inclusive linting tool

For all pull requests

  • Link to the issue you are solving (so it gets closed)
  • Label the pull request with the appropriate area(s)
  • Assign potential reviewers (you may also want to contact them on Teams to ensure timely reviews)
  • Assign the project - Engineering Playbook Backlog
  • Assign the pull request to yourself

Copy link
Member

@kevinleung23 kevinleung23 left a comment

Choose a reason for hiding this comment

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

I love the addition of this into the playbook, thanks for writing this up. I appreciate that you list the benefits and point the developers to the source to learn more.

@jeremydelacruz jeremydelacruz marked this pull request as ready for review October 18, 2021 20:51
@@ -0,0 +1,37 @@
# Inclusive Linting
Copy link
Contributor

Choose a reason for hiding this comment

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

Love to see this guidance for CI. Thanks to propose and create the document.

Copy link
Contributor

@fnocera fnocera left a comment

Choose a reason for hiding this comment

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

Looks great @jeremydelacruz! I would also look for somewhere we can link to this document from within the main CI guidance page.

Copy link

@mkcomer mkcomer left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for adding CI section. Suggested an additional link - action for woke w/ review dog

@jeremydelacruz
Copy link
Contributor Author

jeremydelacruz commented Oct 19, 2021

@fnocera @madossan01 thanks for the reviews! The markdown-link-check step seems to keep failing on the following two links from ./docs/source-control/README.md (though they seem to be working links):

They're referenced in docs unrelated to my changes, but I wanted to flag it. Do we think I should hold off on merging?

@jeremydelacruz jeremydelacruz merged commit 0faa708 into main Oct 20, 2021
@jeremydelacruz jeremydelacruz deleted the jdelacruz/inclusive-linting branch October 20, 2021 12:57
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.

5 participants