xCDAT / xcdat

An extension of xarray for climate data analysis on structured grids.
https://xcdat.readthedocs.io/en/latest/
Apache License 2.0
101 stars 11 forks source link

[PR]: Simplify the contributing guide #593

Closed tomvothecoder closed 1 month ago

tomvothecoder commented 5 months ago

Description

Todo list

Checklist

If applicable:

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (07bfbba) to head (57a9a00).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #593 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 15 15 Lines 1542 1542 ========================================= Hits 1542 1542 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tomvothecoder commented 5 months ago

Hey @xCDAT/core-developers, here is my initial PR draft to simplify the contributing guide. It is based on Xarray's contributing guide, but geared towards xCDAT and made even simpler.

If any of you are available for a quick review, that would be great. You can review the page directly in this Read The Docs preview: https://xcdat.readthedocs.io/en/update-contributing-guide/contributing.html. Thanks!

Here is the old version if you wanted to compare against it: https://xcdat.readthedocs.io/en/v0.6.0/contributing.html. I removed overly technical sections about version control/Git and pre-commit, re-organized and simplified sections such as setting up a dev env, and made it more friendly sounding (less stringent guidelines and standards).

tomvothecoder commented 4 months ago

Hi @chengzhuzhang, just tagging you for review on this PR. It's not urgent, so whenever you have time for it that would be great.

tomvothecoder commented 3 months ago

@tomvothecoder – I did make some changes locally (from my forked repo). I basically just deleted mamba stuff (conda now uses the mamba solver by default).

@pochedls Thanks for the reminder about this update to Conda's default solver. In this commit I added your suggestions and updated the Installation page to change mamba references back to conda.

I had some git setup steps (I tried to start from scratch), e.g.,:

git config --global user.name “…”
git config --global user.email “…”

But I think that can be beyond the scope of our developer guide. We may need to help some of our users with git stuff.

I added a section on setting up the repo by either cloning or forking (this commit).

My thinking was that adding git content would make the contributing guide lengthy. From my experience, the basic commands are easy to look up (configs, cloning, fetching, pulling, pushing, and rebasing). As an alternative, we can help individual contributors directly if they have issues with Git.

tomvothecoder commented 1 month ago

@pochedls Just bugging you again for your approval

tomvothecoder commented 1 month ago

Additional item from today's meeting:

tomvothecoder commented 1 month ago
  • Doesn't the first code block do (1) and (2)? Should the following be outside the code block?:

(1) does not do (2). (1) Creates the dev environment with all of the dependencies required xCDAT, while (2) builds and installs the local version of xcdat into the dev environment since we don't include the stable version of xcdat in the dev env (e.g., xcdat=0.7.0). (2) allows the user to test/try code changes in the env.

pochedls commented 1 month ago

Doesn't make install # or python -m pip install . in the code block under 1. Install the build dependencies actually build/install xcdat?

It seems like the code block under 2. Build and install xcdat simply ensures that it was built correctly and shows up as a loadable module (i.e., it does not build/install xcdat, which was done under buller 1)?

Note that I am looking at this rendered page.

tomvothecoder commented 1 month ago

Doesn't make install # or python -m pip install . in the code block under 1. Install the build dependencies actually build/install xcdat?

It seems like the code block under 2. Build and install xcdat simply ensures that it was built correctly and shows up as a loadable module (i.e., it does not build/install xcdat, which was done under buller 1)?

Note that I am looking at this rendered page.

Ohhh I see what you're saying. (2) should be labeled "Import the local build of xCDAT". Thanks for catching this.

tomvothecoder commented 1 month ago

I addressed the suggestions and will merge this PR now.