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

Open Git Hub Questions from Data Extraction Team #345

Open
HeatherAck opened this issue Aug 15, 2023 · 4 comments
Open

Open Git Hub Questions from Data Extraction Team #345

HeatherAck opened this issue Aug 15, 2023 · 4 comments
Assignees
Labels
help wanted Extra attention is needed

Comments

@HeatherAck
Copy link
Contributor

HeatherAck commented Aug 15, 2023

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?

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

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

  3. 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
Copy link
Contributor Author

@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.

@eb-oss
Copy link
Contributor

eb-oss commented Aug 17, 2023

  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.

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

  2. 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
Copy link

heshiming commented Aug 17, 2023

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.

@eb-oss eb-oss moved this from Todo to In Progress in Data Commons Platform Aug 23, 2023
@DaBeIDS
Copy link

DaBeIDS commented Aug 31, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
Status: In Progress
Development

No branches or pull requests

8 participants