-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Detects if master branch has diverged and warn to pull new commits #815
Conversation
Codecov Report
@@ Coverage Diff @@
## master #815 +/- ##
==========================================
- Coverage 71.83% 71.82% -0.02%
==========================================
Files 65 65
Lines 5213 5203 -10
==========================================
- Hits 3745 3737 -8
+ Misses 1183 1181 -2
Partials 285 285
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, I think this is a good approach. You need to add tests for both working_dir and markdown_renderer that exercise the new functionality.
Hi, |
Hi @lkysow, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work? I'm seeing that the status
is just looking at the remote tracking branch which isn't master
but the same branch:
$ git status
On branch autoplan
Your branch is up to date with 'origin/autoplan'.
nothing to commit, working tree clean
Even when master
is ahead.
} | ||
status := strings.Trim(string(outputStatusUno), "\n") | ||
hasDiverged := strings.Contains(status, "have diverged") | ||
if hasDiverged { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we skip this section if any of the above commands fail? Maybe pull it into its own function so you can return early.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it like this because I don't want to impact the clone function if for any reason it cannot detected that master branch has diverged as it displays only a warning in the comment of the plan.
Yes this is working, did you tried with argument I got this:
|
Closed by #867 (I took all your commits and made some small changes, awesome work! 🎉 ) |
Hi Luke,
I've worked on implementing a warning in the comment if master has changed like as I suggested in issue #804.
There is an example at MRinalducci/atlantis-example#1 (comment).
It adds message: ":warning: Master branch is ahead, it is recommended to pull new commits first."
Can you please give me your inputs about this PR?
I think about to implement a "pull/merge" command. What do you think?