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

GitHub confuses me AGAIN; how should I merge changes from master? #2

Open
MichaelTiemannOSC opened this issue Aug 27, 2022 · 5 comments
Assignees

Comments

@MichaelTiemannOSC
Copy link
Contributor

GitHub tells me that my iceberg-dbt branch is 1 commit behind master.

I create the suggested Pull Request to merge the change (the addition of the Apache 2.0 License by @HeatherAck).

This creates a DCO problem (because the license was not checked in correctly?)

I attempt to fix this by following instructions that result in an absolutely empty commit. (Which elsewhere GitHub helpfully warns me will wreak future havoc on further attempts to correct "simple" check-in and branch edits caused by inevitable DCO problems.)

So now I have effectively done nothing, except to put my branch in a more precarious position that it was in previously.

The real problem is likely that I created a branch without the proper upstream tagging and GitHub is merely throwing random suggestions at me that are at most unhelpful. How many others are having the same troubles? Does it make sense to "look for trouble" via some automated search of our repos?

@eb-oss
Copy link

eb-oss commented Aug 29, 2022

Ok, there are a lot of issues here. First, none of the previous commits were made according to the proper Github fork-and-PR standards. If they had been, the lack of DCO on those would've been caught (not to mention that Heather's last commit has an example command in the commit message body, which is exactly the sort of thing that even a cursory review should catch. But since that's the last commit, it's easy enough to fix).

Second, you should not open pull requests to your feature branches. PRs should be opened to the main/master branch only (or, occasionally, a release branch). A feature branch should be rebased from the main branch. Changes shouldn't be merged in, and you should definitely not open PRs to feature branches.

So you'll want to do a rebase, but first, we should clean up that last commit from Heather. @HeatherAck could you please reach out to me on Slack? We can go through the process of fixing that commit message, adding a DCO, and then rewriting the top commit with a fixed version. @MichaelTiemannOSC I'll update once that's done, and then we can see if you have any issues with rebasing from master.

@eb-oss
Copy link

eb-oss commented Aug 29, 2022

A couple quick additions I forgot in my initial message:
I have an open change for improving our git docs, so this might be a good experience for seeing things that could be added there. You can see the current change here: https://github.com/os-climate/OS-Climate-Community-Hub/pull/53/files

Also, for git issues like this, you can just tag me. Ryan works exclusively on the infra side of things, so when it comes to git issues related to branches, merging, commits, DCO, etc, he doesn't need to be included.

@MichaelTiemannOSC
Copy link
Contributor Author

Thanks for that--and I totally get it. See you over on Slack in a few minutes to address, and thanks for advise on assignees.

@MichaelTiemannOSC
Copy link
Contributor Author

OK...I am now ready to work via Slack for the next few hours...

@eb-oss
Copy link

eb-oss commented Aug 29, 2022

After discussion on Slack, Heather is OOO currently, so we'll wait until she returns to get that HEAD commit fixed. Since it's not strictly necessary to have the feature branch up to date (no changes are being made to the LICENSE file, so there shouldn't be any conflicts introduced), it shouldn't be a problem to hold off until Heather returns.

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

No branches or pull requests

3 participants