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

Refactor release modules #80

Merged
merged 7 commits into from
Apr 16, 2021
Merged

Refactor release modules #80

merged 7 commits into from
Apr 16, 2021

Conversation

bjoernricks
Copy link
Contributor

@bjoernricks bjoernricks commented Apr 15, 2021

What:

Update the release module to be more precise about which parameters the
commands prepare, release and sign require. This will simplify using
pontos release.

Why:

Be more explicit about which parameter is required for which command and therefore show a better --help

How:

Checklist:

Update the release module to be more precise about which parameters the
commands prepare, release and sign require. This will simplify using
pontos release.
Re-arange tests into three different testcase classes reflecting the
commands prepare, release and sign.
@bjoernricks bjoernricks marked this pull request as ready for review April 15, 2021 11:53
@bjoernricks bjoernricks requested a review from a team April 15, 2021 11:53
@codecov
Copy link

codecov bot commented Apr 15, 2021

Codecov Report

Merging #80 (e52b66e) into master (59bf052) will increase coverage by 0.08%.
The diff coverage is 99.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #80      +/-   ##
==========================================
+ Coverage   94.05%   94.14%   +0.08%     
==========================================
  Files          29       29              
  Lines        1514     1502      -12     
==========================================
- Hits         1424     1414      -10     
+ Misses         90       88       -2     
Impacted Files Coverage Δ
tests/release/test_release.py 98.86% <98.21%> (-0.05%) ⬇️
pontos/release/release.py 97.14% <100.00%> (+1.03%) ⬆️

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 59bf052...e52b66e. Read the comment docs.

pontos/release/release.py Outdated Show resolved Hide resolved
pontos/release/release.py Outdated Show resolved Hide resolved
pontos/release/release.py Outdated Show resolved Hide resolved
git_space=release_info.space,
release_version,
project,
git_tag_prefix=git_tag_prefix,
Copy link
Member

Choose a reason for hiding this comment

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

There is (now) already a default value in the argparser, so in add_skeleton we probably do not need the default value anymore. (https://github.com/greenbone/pontos/blob/master/pontos/changelog/changelog.py#L50)
(Same for the git_space)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

personally i would keep it because these are independent modules.

help='Will release changelog as version. Must be PEP 440 compliant',
required=True,
)
sign_parser.add_argument(
Copy link
Member

Choose a reason for hiding this comment

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

Since this argument is in all subparsers. Why not add it to the parent parser and save some code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Personally i wanted to avoid --release-version foo prepare --next-version bar because it wont be possible to use prepare --release-version foo --next-version bar otherwise.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah.Ok!

pontos/release/release.py Outdated Show resolved Hide resolved
pontos/release/release.py Outdated Show resolved Hide resolved
pontos/release/release.py Outdated Show resolved Hide resolved
The main requests module already provides the request api.
Add missing typings for the return values of prepare, release and sign
functions.
Use a single constant variable to the release file name. This allows for
easier changing the file name and avoids typos.
@bjoernricks bjoernricks merged commit 8b6c85a into greenbone:master Apr 16, 2021
@bjoernricks bjoernricks deleted the refactor-release-module branch April 16, 2021 12:06
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