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

Rebase the service-vm branch instead of patches #23

Merged
merged 1 commit into from Dec 6, 2018

Conversation

tomassedovic
Copy link
Collaborator

Applying patches from the service-vm branch now fails because there's
more than one patch we need (and as far as I can tell, github doesn't
let us specify that).

So we could either curl and apply the individual commits (which means
updating ocp-doit every time we make a change to that branch) or just
checking the branch and merging it in.

This change does the latter approach as it's more future-proof.

https://github.com/openshift/installer.git

# NOTE(shadower): we can't just just download and apply the patch here -- the branch has several commits
pushd "$GOPATH/src/github.com/openshift/installer"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other option is to apply the 3 patches. We can get a patch file per commit. It'd be pretty much like it was before but all coming from the same branch. what do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I've mentioned that in the commit message. That would mean any change to this branch would require changing ocp-doit to update the commits.

I'm not keen, but we can do that instead, sure.

A third option would be to squash the service-vm commits in your branch. But only you can do that I think.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's squash, lemme do that

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

squashed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one, thanks!

All changes from that patch were now squashed to flaper87's `service-vm`
commit so applying it now would cause git conflicts.
@tomassedovic
Copy link
Collaborator Author

Ok with the commits squashed, this PR is just about removing my earlier DNS patch, but it's not applying cleanly it would seem :-(

@tomassedovic
Copy link
Collaborator Author

Ok, I made a mistake. This change actually passes fine, but there's another thing we've got broken later in the ocp_repo_sync script:

sync_go_repo_and_patch sifs.k8s.io/openshift/cluster-api-provider-openstack https://github.com/openshift/cluster-api-provider-openstack.git

I'll merge this PR and we'll need a rebase for cluster-api-provider-openstack.

@tomassedovic tomassedovic merged commit 7d992bb into master Dec 6, 2018
@tomassedovic tomassedovic deleted the merge-service-vm branch December 6, 2018 08:47
@tomassedovic
Copy link
Collaborator Author

@flaper87 see

sync_go_repo_and_patch sifs.k8s.io/openshift/cluster-api-provider-openstack https://github.com/openshift/cluster-api-provider-openstack.git

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.

None yet

2 participants