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

nox: add docstring about nox and list command how to use it #3254

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

consideRatio
Copy link
Contributor

@consideRatio consideRatio commented Oct 11, 2023

I think its a great practice to ensure that various things in a github repo is somewhat self-explanatory as that helps anyone trying to onboard do it more efficiently. I didn't know how to use nox myself before doing this, and at one point I didn't know for what reason this file was around or how it was related to a repo I found it in.

A docstring like this can resolve such issues for us!

@consideRatio consideRatio requested a review from a team as a code owner October 11, 2023 13:10
@consideRatio consideRatio added the nominated-to-be-resolved-during-q4-2023 Nomination to be resolved during q4 goal of reducing the technical debt label Oct 11, 2023
Copy link
Member

@choldgraf choldgraf left a comment

Choose a reason for hiding this comment

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

This is a great idea. I'm kicking myself for not adding in a little docstring. I think the content looks great and I'd be fine with merging as-is, though I also wonder what @consideRatio thinks about simply linking to our nox documentation in the team compass:

https://compass.2i2c.org/reference/documentation/environment/#install-nox

(or potentially making that a dedicated section on Nox so that it's more discoverable). That way, we can point all of our noxfile.py files to that site, and update the site's content without needing to update each individual file.

@consideRatio
Copy link
Contributor Author

consideRatio commented Oct 12, 2023

Thanks for reviewing and approving it as it is @choldgraf!!! ❤️ 🎉

For this brief intro that I think by itselfs does a good enough job, I think its a good call to break the DRY principle and directly write it all noxfile.py files.

If we were to link to a centralized intro, I still think it would make sense to provide a brief intro like this before linking out as otherwise it may not be enticing enough to click the link in the first place.

@consideRatio consideRatio merged commit 88e9d14 into 2i2c-org:master Oct 12, 2023
2 checks passed
@consideRatio consideRatio added the selected-to-be-resolved-during-q4-2023 Selected to be resolved during q4 goal of reducing the technical debt by goal lead label Oct 12, 2023
@consideRatio
Copy link
Contributor Author

consideRatio commented Oct 12, 2023

Ah missed that there was existing docs to link to already.

I think adding a link to that could make sense to learn a bit more, but that the first paragraph and config ref link is worth retaining inlined in the file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nominated-to-be-resolved-during-q4-2023 Nomination to be resolved during q4 goal of reducing the technical debt selected-to-be-resolved-during-q4-2023 Selected to be resolved during q4 goal of reducing the technical debt by goal lead
Projects
No open projects
Status: Done 🎉
Development

Successfully merging this pull request may close these issues.

Add description about what noxfile.py is
2 participants