Skip to content
This repository has been archived by the owner on Jan 12, 2021. It is now read-only.

Change --tag option from bool to template in project:bump command #199

Merged
merged 9 commits into from
Jul 3, 2019

Conversation

b0g3r
Copy link
Contributor

@b0g3r b0g3r commented Jul 1, 2019

Closes #197

I have questions about three things:

  1. CONTRIBUTE.md doesn't say anything about running modified dephell for manual testing, only running checking and testing. Now I'm using poetry run dephell command, but I'm not sure about it.
  2. I don't see any tests for argument schema validation. Why?
  3. I have no idea about git tag creation testing :( I tried to search, but found only tests for git package resolving.

@orsinium
Copy link
Member

orsinium commented Jul 1, 2019

Beautiful. Thank you!

I think, we can use {version} instead of {new_version}. Also, if placeholder is not specified, we can interpret it as a prefix. For example, --tag="v."

@b0g3r
Copy link
Contributor Author

b0g3r commented Jul 1, 2019

Good proposal, I implemented the prefix feature! But I concerned about testing git tag feature, how can I test it?

@orsinium
Copy link
Member

orsinium commented Jul 1, 2019

Oh, that's a long story. We have to make temporary directory, git init, run command and check commits. However, we have no this kind of tests yet, it's in future plans. I've commented it in #167, and then we lost contributor. So, I can accept PR without tests and add it in the scope of future works about this kind of tests.

@b0g3r b0g3r marked this pull request as ready for review July 1, 2019 21:51
@b0g3r
Copy link
Contributor Author

b0g3r commented Jul 2, 2019

Added tests!

Copy link
Member

@orsinium orsinium left a comment

Choose a reason for hiding this comment

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

Wonderful! It's a big step to the perfect tests and important contribution.

I've added a few comments on how to make it a little bit better. However, important only 2 of them: about the error message and about documentation. It's the things that users see. Everything isn't really important, but it would be cool if you've done it BTW.

docs/cmd-project-bump.md Show resolved Hide resolved
dephell/commands/project_bump.py Outdated Show resolved Hide resolved
dephell/commands/project_bump.py Outdated Show resolved Hide resolved
dephell/commands/project_bump.py Outdated Show resolved Hide resolved
@orsinium
Copy link
Member

orsinium commented Jul 2, 2019

Perfect, thank you :) Tonight Travis CI has troubles with Linux build, so, I'll merge it tomorrow, if tests works

@orsinium orsinium merged commit 0bb9d55 into dephell:master Jul 3, 2019
@orsinium
Copy link
Member

orsinium commented Jul 3, 2019

Thank you, nice job.

If you'll have time and willing to help more, we have a bug #127 related to the usage of .git in some tests. If you propagate the way of testing from this PR on other git-based commands, it will work. Feel free to contact me or open an issue if you have some questions or bugs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add tag prefix to settings for project bump
2 participants