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

Updating message when project is locked. #325

Merged

Conversation

marcb1
Copy link
Contributor

@marcb1 marcb1 commented Oct 23, 2018

Just updating the message atlantis spits out when the project is locked. Once the locking plan is released, atlantis doesn't auto-run plan on subsequent PR's. This message makes it clear to users that they should manually run plan.

@marcb1 marcb1 force-pushed the updating-message-when-project-is-locked branch 2 times, most recently from 847d839 to 9b5a967 Compare October 23, 2018 15:08
@codecov
Copy link

codecov bot commented Oct 23, 2018

Codecov Report

Merging #325 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #325      +/-   ##
==========================================
+ Coverage   70.56%   70.57%   +<.01%     
==========================================
  Files          61       61              
  Lines        3642     3643       +1     
==========================================
+ Hits         2570     2571       +1     
  Misses        893      893              
  Partials      179      179
Impacted Files Coverage Δ
server/events/project_locker.go 90% <100%> (+0.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f56e06a...189b059. Read the comment docs.

Copy link
Member

@lkysow lkysow left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I agree this should be a better message. I've suggested another change to make it even more explicit.

@@ -63,7 +63,7 @@ func (p *DefaultProjectLocker) TryLock(log *logging.SimpleLogger, pull models.Pu
}
if !lockAttempt.LockAcquired && lockAttempt.CurrLock.Pull.Num != pull.Num {
failureMsg := fmt.Sprintf(
"This project is currently locked by #%d. The locking plan must be applied or discarded before future plans can execute.",
"This project is currently locked by #%d. The locking plan must be applied or discarded before future plans can execute. Comment with `atlantis plan` on this PR, once the lock is released.",
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to change this so it looks like:

Unable to run plan. This project is currently locked by an unapplied plan from pull #2. To continue, delete the lock from #2 or apply that plan and merge the pull request.

Once the lock is released, comment atlantis plan here to re-plan.

The full string will be:

fmt.Sprintf("**Unable to run `plan`**. This project is currently locked by an unapplied plan from pull #%d. To continue, delete the lock from #%d or apply that plan and merge the pull request.\n\nOnce the lock is released, comment `atlantis plan` here to re-plan.", lockAttempt.CurrLock.Pull.Num, lockAttempt.CurrLock.Pull.Num)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cool, updated!

@marcb1 marcb1 force-pushed the updating-message-when-project-is-locked branch 4 times, most recently from 2a6a74e to 7010971 Compare October 26, 2018 17:24
@marcb1 marcb1 force-pushed the updating-message-when-project-is-locked branch from 7010971 to 189b059 Compare October 26, 2018 18:03
@lkysow
Copy link
Member

lkysow commented Oct 30, 2018

image

Looks great!

@lkysow lkysow merged commit 5a8230d into runatlantis:master Oct 30, 2018
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.

2 participants