Skip to content

Do not checkout the branch passed to CommitFileViaConsole #41

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

Merged

Conversation

weierophinney
Copy link
Member

We need to assume we're on the correct branch; otherwise, a checkout operation will lead to an error, aborting the operation.

Instead, I've added functionality to verify that we are on the branch provided; if not, an exception is raised.

The changes also required annotating that the repository directory, filename, and commit message arguments could not be empty, which required changes elsewhere to ensure this is true before calling on CommitFile.

@weierophinney weierophinney added this to the 1.2.0 milestone Aug 12, 2020
@weierophinney weierophinney added the Bug Something isn't working label Aug 12, 2020
weierophinney added a commit to weierophinney/automatic-releases that referenced this pull request Aug 12, 2020
We need to assume we're on the correct branch; otherwise, a checkout
operation will lead to an error, aborting the operation.

Instead, I've added functionality to verify that we are on the branch
provided; if not, an exception is raised.

The changes also required annotating that the repository directory,
filename, and commit message arguments could not be empty, which
required changes elsewhere to ensure this is true before calling on
`CommitFile`.

Signed-off-by: Matthew Weier O'Phinney <[email protected]>
Signed-off-by: Matthew Weier O'Phinney <[email protected]>
@weierophinney weierophinney force-pushed the hotfix/do-not-checkout-to-commit branch from fc96c12 to 55057b9 Compare August 12, 2020 15:37
Renamed `assertBranch` to `assertWeAreOnBranch`. Additionally, instead
of:

- Process:run
- Process:isSuccessful
- throw

use just `Process::mustRun()`

Signed-off-by: Matthew Weier O'Phinney <[email protected]>
@weierophinney
Copy link
Member Author

Feedback incorporated!

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Ocramius Ocramius self-assigned this Aug 12, 2020
@Ocramius Ocramius merged commit 1e630f7 into laminas:1.2.x Aug 12, 2020
@weierophinney weierophinney deleted the hotfix/do-not-checkout-to-commit branch August 12, 2020 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants