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

Add contributing.md #463

Merged
merged 5 commits into from
Apr 2, 2024
Merged

Add contributing.md #463

merged 5 commits into from
Apr 2, 2024

Conversation

BradyAJohnston
Copy link
Owner

Adds some documentation for contributing. Plenty to still include and expand upon.

Copy link

codecov bot commented Mar 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.74%. Comparing base (6e82364) to head (bef9446).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #463   +/-   ##
=======================================
  Coverage   77.74%   77.74%           
=======================================
  Files          40       40           
  Lines        3612     3612           
=======================================
  Hits         2808     2808           
  Misses        804      804           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kainszs
Copy link
Contributor

kainszs commented Mar 31, 2024

Have you ever heard of pre-commit-hooks? That could simplify your work. I've seen that you format your code with black by default. The pre-commit hooks would do this before pushing. You can also configure them so that the tests run through, or set ruff as a linter before pushing.

@BradyAJohnston
Copy link
Owner Author

I do use ruff as a linter locally, but I haven't enabled it on GHA yet as there is a lot that it would pick up. Have been meaning to for quite some time go through and do a general cleanup & refactor.

@BradyAJohnston
Copy link
Owner Author

I haven't used pre-commit hooks, but would certainly be open to adding them.

@kainszs
Copy link
Contributor

kainszs commented Mar 31, 2024

Try working with it locally, it can be annoying with many small commits, because it tests the changed files and changes them if necessary, so that you have to commit again. It can also be tedious to set it up once, as it is best practice during installation to run the pre-commit hooks on all files.

If you like it, we can also set it up together. The great added value that I see, especially in open source projects with many contributors, is that the contributed code is already tested locally and formatted if necessary, so that there are no more commits than necessary.

@kainszs
Copy link
Contributor

kainszs commented Mar 31, 2024

It might also be a good idea to offer a second requirements.txt that contains the additional functionality for the contributors. This would also be maintainable via the GHA without additional effort.

@rbdavid
Copy link
Contributor

rbdavid commented Mar 31, 2024

I found that a slight rearrangement of the MN directory tree helped me work on the GUI panels and live test them in a Blender instance. I moved the ~/molecularnodes/ directory to ~/addons/molecularnodes/. Then, in the Blender Settings -> File Paths -> Script Directories, I added the path to the git repo's home directory, which enables Blender to see the git repo's addon directory so that molecularnodes can be loaded from there. With this setup, when I make changes to the molecularnodes code, I can reload the MN addon in a Blender instance to test the code. I can also git checkout to other branches as needed (e.g. I have projects working with MN v4.0.9).

Without this rearrangement, once having made changes to MN code, I would have to copy those altered files to a MN version that is importable as a Blender addon. This is just a small nuisance but has led me to screwing up a few times, where I was left head-scratching about code when I had just forgotten to do the copy. Maybe you all have a better approach to this?

The one issue that I think might arise is whether git-checkout can detect changes in files like the MN_data_file.blend that happen between versions. I think adding versions to these file names would avoid file conflicts.

@BradyAJohnston
Copy link
Owner Author

@rbdavid with the Blender VSCode plugin, you can set it so that every time you save a file it refreshes the add-on. This is how I have it set up so every time I make a change it updates the GUI straight away.

@kainszs
Copy link
Contributor

kainszs commented Apr 1, 2024

I like the explanations very much and they are an excellent starting point.

@BradyAJohnston BradyAJohnston merged commit ccb6a64 into main Apr 2, 2024
7 checks passed
@BradyAJohnston BradyAJohnston deleted the contributing branch April 2, 2024 06:27
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.

3 participants