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.

Sign up for GitHub

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

Jump to bottom

fix: push modifications to boot repo directly to master #5756

Merged
merged 8 commits into from
Oct 10, 2019
Merged

fix: push modifications to boot repo directly to master #5756

merged 8 commits into from
Oct 10, 2019

Conversation

hferentschik
Copy link

@hferentschik hferentschik commented Oct 9, 2019 •
edited
Loading

fixes #5672

NOTE: You won't be able to re-run jx boot after the first bootstrap from the initial boot config directory. You will get errors of the form:

Attempting to cherry pick commits that were on master but not yet pushed
WARNING: Unable to cherry-pick 1c384abadb71921d01971922930decfecc03e6f3: git output: error: could not apply 1c384ab... initial config based of hf-bee-bot/environment-hardy-bdd-dev-test with ref v1.0.25
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit': failed to run 'git cherry-pick 1c384abadb71921d01971922930decfecc03e6f3' command in directory '/Users/hardy/tmp/dummy-boot/jenkins-x-boot-config', output: 'error: could not apply 1c384ab... initial config based of hf-bee-bot/environment-hardy-bdd-dev-test with ref v1.0.25
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit''

I'll create a new issue for that. The problem even exists already exists in master as it stands now, so it is not new.

Here is the follow up issue - #5772

@hferentschik hferentschik force-pushed the issue-5672 branch 2 times, most recently from 3afe96a to 43cb736 Compare October 9, 2019 13:17
@hferentschik hferentschik requested review from pmuir, dgozalo and daveconde and removed request for garethjevans and rawlingsj October 9, 2019 14:58
@hferentschik hferentschik force-pushed the issue-5672 branch 2 times, most recently from 38f6373 to 024639f Compare October 9, 2019 15:54
hferentschik
hferentschik commented Oct 9, 2019
View reviewed changes
pkg/cmd/step/verify/step_verify_environments.go
return errors.Wrapf(err, "resolving %s to absolute path", o.Dir)
}

environmentRepo, err := o.createDevEnvironmentRemote(envGitInfo, dir, fromGitURL, fromBaseRef, !public, requirements, provider, gitter)
Copy link
Author

Choose a reason for hiding this comment

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

@pmuir one thing I don't understand is why we create the remote and push the original repo data to it, but ony in the jx boot case add a commit with the modifications?

I also find it odd that we push something to the remote, then fork and pull, add a commit and then push again. Could we not do everything locally on then do a single push?

Apart from this, afaict the refactored code does not what the original code did, except that the changes to config are pushed to master instead of creating a pull request.

Still cannot shake off the feeling that we only should push once.

@hferentschik hferentschik force-pushed the issue-5672 branch 2 times, most recently from 48923a7 to 9a9611a Compare October 10, 2019 08:58
@hferentschik
fix: removing unused cmd options lazy create and env dir
1cb1646
@hferentschik
chore: fix godoc for DuplicateGitRepoFromCommitish
732724f
@hferentschik
fix: push modifications to boot repo directly to master
d522739 
- when using `jx boot` with gitops enabled there is no PR generated anymore for
  the initial changes to the boot config.
- moving check for intepret mode into StepVerifyPreInstallOptions#IsJXBoot
- introduce StepVerifyEnvironmentsOptions#readEnvironment and StepVerifyEnvironmentsOptions#isJXBoot

fixes jenkins-x#5672
@hferentschik
fix: squash all commits of initial boot config
32748ed
@hferentschik hferentschik force-pushed the issue-5672 branch 2 times, most recently from 315f6dc to 6715b40 Compare October 10, 2019 12:10
@hferentschik hferentschik changed the title [wip] fix: push modifications to boot repo directly to master fix: push modifications to boot repo directly to master Oct 10, 2019
@abayer
Copy link
Contributor

abayer commented Oct 10, 2019

/lgtm

@jenkins-x-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abayer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jenkins-x-bot jenkins-x-bot merged commit ced5256 into jenkins-x:master Oct 10, 2019
@hferentschik hferentschik deleted the issue-5672 branch October 11, 2019 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@pmuir pmuir Awaiting requested review from pmuir

@dgozalo dgozalo Awaiting requested review from dgozalo

@daveconde daveconde Awaiting requested review from daveconde

@abayer abayer Awaiting requested review from abayer

Assignees

@abayer abayer

Labels
approved lgtm size/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Initial 'jx boot' execution should push config directly to master
4 participants
@hferentschik @abayer @jenkins-x-bot @houndci-bot