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

Add warning on publish if no branch was found. #795

Merged
merged 1 commit into from
Mar 13, 2019

Conversation

Alphare
Copy link
Contributor

@Alphare Alphare commented Mar 6, 2019

When the history has been modified (either with draft changesets in hg, or with commit rebasing in git), it is possible that no corresponding branch be found when publishing. There is no real way of automatically fixing the issue, so we at least have to log a warning.

However, it does not seem like there are tests for logs except the console tests for indentation, and I feel like this change warrants one.
Do you have a preferred method of going about this?

@Alphare Alphare force-pushed the missing_branch_publish branch from 90c3081 to c64af17 Compare March 6, 2019 11:14
@Alphare
Copy link
Contributor Author

Alphare commented Mar 6, 2019

I've just amended my commit with an improvement to speed and readability by extracting the list of branches for the current commit out of the loop.

results.commit_hash in commits]

# Print a warning message if we couldn't find the branch of a commit
if not len(branches_for_commit):
Copy link
Contributor

Choose a reason for hiding this comment

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

len() is useless here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figured this was more explicit that way, it's almost a question of code style. If you think it's cleaner, I'll remove it

Copy link
Collaborator

Choose a reason for hiding this comment

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

len() is not recommended

asv/commands/publish.py Outdated Show resolved Hide resolved
When the history has been modified (either with draft changesets in hg,
or with commit rebasing in git), it is possible that no corresponding
branch be found when publishing. There is no real way of automatically
fixing the issue, so we at least have to log a warning.
@Alphare Alphare force-pushed the missing_branch_publish branch from c64af17 to 865f0cc Compare March 6, 2019 23:04
@pv
Copy link
Collaborator

pv commented Mar 13, 2019

If you want to add general tests for logging, that's of course welcome.
However, perhaps best to add those in a separate PR.

Otherwise the change here looks ok, modulo the style comment.

@pv pv merged commit 7afea8e into airspeed-velocity:master Mar 13, 2019
@pv
Copy link
Collaborator

pv commented Mar 13, 2019

Thanks, merged, not going to hold it for a style issue.
Any tests are welcome in a separate PR.
(The CI seems to have hiccups vs conda and pytest once again...)

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.

3 participants