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

[MISC] add CONTRIBUTING.md #247

Merged
merged 2 commits into from
Dec 24, 2024
Merged

Conversation

ziyanx02
Copy link
Collaborator

Add CONTRIBUTING.md.

@YilingQiao
Copy link
Collaborator

Hi, could you please test if the pre-commit hook is working as expected? I want to set the line width to 120, but it seems to format everything with a smaller value.

@ziyanx02
Copy link
Collaborator Author

Hi, could you please test if the pre-commit hook is working as expected? I want to set the line width to 120, but it seems to format everything with a smaller value.

I will try it tomorrow

@zswang666
Copy link
Collaborator

Should we add guide for test/check in CI on local machine?

black --check .
black . # to auto-format if the above check failed
python -m unittest discover tests

@ziyanx02
Copy link
Collaborator Author

Seems those CI tests require >6G gpu memory so I will add local tests as optional.

As for auto-reformat, I got 19 files to be reformatted after running black --check . XD. That doesn't seem right, does it?

@ziyanx02
Copy link
Collaborator Author

ziyanx02 commented Dec 24, 2024

Hi, could you please test if the pre-commit hook is working as expected? I want to set the line width to 120, but it seems to format everything with a smaller value.

I can't find a countercase. An extremely long string or name won't be truncated even longer than 120. Long numerical expressions will be divided into parts as short as possible I guess? I can't tell the reason why a line is reformatted.

@ziyanx02
Copy link
Collaborator Author

ziyanx02 commented Dec 24, 2024

The final version is

  • Use pre-commit hooks to ensure you don’t forget to run checks.
  • Suggesting to do local CI tests locally.

@ziyanx02 ziyanx02 merged commit 257ca2c into Genesis-Embodied-AI:main Dec 24, 2024
1 check passed
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.

4 participants