os-climate / os_c_data_commons

Repository for Data Commons platform architecture overview, as well as developer and user documentation
Apache License 2.0
18 stars 10 forks source link

Open Git Hub Questions from Data Extraction Team #345

Open HeatherAck opened 11 months ago

HeatherAck commented 11 months ago

Github Questions:

1) Lets assume we had a hotfix on main branch. In the following link (https://nvie.com/posts/a-successful-git-branching-model/) they explain that afterwards we can merge the hotfix of main into main and then into develop branch via:

$ git checkout master Switched to branch 'master' $ git merge --no-ff hotfix-1.2.1 Merge made by recursive. (Summary of changes) $ git tag -a 1.2.1

And then

$ git checkout develop Switched to branch 'develop' $ git merge --no-ff hotfix-1.2.1 Merge made by recursive. (Summary of changes)

But afterwards the status of develop looks like this:

Image

I guess the 1 ahead is the merge of hotfix to develop and the behind is the one hotfix to main. My idea would be to actually not do that but to do the following:

MY APPROACH: git checkout main git merge --signoff --no-ff hotfix git tag -a 0.2 git push

git checkout develop git merge --ff-only main git push

Then develop and main are up-to-date. Is that procedure ok? Or can somebody explain or give a link how to do that correctly?

2) Is there an easy way to add a CI layer to our repository [corporate_data_extraction]?

3) Has somebody changed the repo? Tobias has no access to his branch anymore.

4) Is there a way to prevent that people merge to main branch and also that pull-requests have to be combined with issues so that one always has a documentation?

HeatherAck commented 11 months ago

@MightyNerdEric and @ModeSevenIndustrialSolutions - can you please provide your recommendations re: questions 1, 2, and 4?

@DaBeIDS, re: #3 -@MichaelTiemannOSC updated permissions to correct a more global problem where some project members were not defaulting to read only. Tobias was likely caught in that update. I will create a new issue to give him write access.

MightyNerdEric commented 11 months ago

1) That guide, as it mentions at the top, is quite old and outdated. And while much of the advice is still fine, I would never suggest doing things the exact way they lay out for the hotfix. You do not want to do a no-ff merge into both master/main and develop, because this creates two different merge commits, which causes the branches to diverge.

The proper way to do this will depend on how you are doing releases/versioning, but I think the proper way to do it where it will be picked up by both main and develop is to open a PR to main (which will give you the merge commit), and then either open a PR into develop from main (which will maintain all merge history, and just end up with some redundant merge commits when it's merged back into main), or manually rebase develop from main and force-push it. The latter would be the cleanest solution, but is not very appropriate for a distributed codebase (this is the kind of thing I might do to bring in an important fix to a feature branch I'm in the middle of working on; if you're doing most of your coding from/merging into develop, opening a PR to get the latest code from main would be the safest way to get it updated.

2) Certainly, we could add Github Actions fairly easily. What are you trying to check?

4) We can add branch protections to main to prevent merging to it. We can also make develop the default branch, so that it will be the default when people open PRs against the repo. There's no setting in Github to require an issue before opening/merging a PR, but this can be accomplished via Github Actions.

heshiming commented 11 months ago

I think we could do this the easy way, maintaining only one stable branch, the main/master. When work on something, create your own branch. Without GitHub, it's like:

... at main branch
$ git clone something && cd something
$ git checkout -b shimingsfe
... at shimingsfe branch
$ git commit (after the work)

At this point, I know that I have the shimingsfe branch following the current main/master. I signal the maintainer that my branch is ready to be merged. The maintainer then does:

... at main branch
$ git merge origin/shimingsfe

If main/master has been changed and git couldn't auto merge. It is usually up to the branch to address conflicts. So I'll do:

... at shimingsfe branch
$ git merge main

This way, I have the main/master branch changes merged into my branch, during which I address conflicts manually. When the merge is complete, $ git merge origin/shimingsfe should have no problem merging this branch.

With GitHub, branches are not needed. I simply "fork" the project into my own. Commit to the main/master to my own fork. Then send a pull request.

In essence, this procedure worked for decades, back to the old CVS/SVN days. We probably don't need a develop branch as I don't envision multiple people working on the same branch. It's much easier to keep only the main/master as stable.

A branch exists for the purpose of being merged. Branches are not to be deleted, unless its purpose/requirements/specs are recalled.

DaBeIDS commented 10 months ago

Dear all, thanks for the very detailed comments, suggestions and explanations. I learned a lot with those comments and the explanations from our last data extraction meeting. In the future it is planned to only maintain one stable branch main and changes/adjustments/improvements should be made via a fork/PR proceedure as described by Shiming at the end of his previous post. We will also add a document in the repo where we explain that to new contributors to the project. Best regards, David @HeatherAck I would not close the issue yet, but wait until we have the mentioned documentation in place, if that is ok for you.