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

[pontos-release]: Remove project parameter, f-string-ed strings, refactor #105

Merged
merged 9 commits into from Jun 8, 2021

Conversation

y0urself
Copy link
Member

@y0urself y0urself commented Jun 6, 2021

What:

  • Remove the --project parameter and resolve the project name with get_project_name()
  • Switch from .format() to f-string in release.py
  • Moved functions into helper.py, to reduce LoC per file ...
  • Adjusted Documentation and added Documentation partly

Why:

How:

Checklist:

@codecov
Copy link

codecov bot commented Jun 6, 2021

Codecov Report

Merging #105 (afa12e2) into master (6333786) will increase coverage by 0.37%.
The diff coverage is 90.57%.

❗ Current head afa12e2 differs from pull request most recent head c7bd967. Consider uploading reports for the commit c7bd967 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #105      +/-   ##
==========================================
+ Coverage   88.34%   88.72%   +0.37%     
==========================================
  Files           9       10       +1     
  Lines         678      683       +5     
  Branches      106      104       -2     
==========================================
+ Hits          599      606       +7     
+ Misses         61       60       -1     
+ Partials       18       17       -1     
Impacted Files Coverage Δ
pontos/version/cmake_version.py 89.74% <25.00%> (ø)
pontos/version/helper.py 77.14% <77.14%> (ø)
pontos/release/release.py 93.61% <95.91%> (-0.47%) ⬇️
pontos/release/helper.py 100.00% <100.00%> (ø)

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 c123a6d...c7bd967. Read the comment docs.

@y0urself y0urself marked this pull request as ready for review June 6, 2021 10:28
@y0urself y0urself requested a review from a team June 6, 2021 10:28
@y0urself y0urself changed the title [pontos-release] Remove project parameter, f-string-ed strings, refactor [pontos-release]: Remove project parameter, f-string-ed strings, refactor Jun 6, 2021
@y0urself y0urself enabled auto-merge June 6, 2021 10:32
pontos/release/helper.py Outdated Show resolved Hide resolved
pontos/release/helper.py Outdated Show resolved Hide resolved
args.append(to)
if develop:
args.append('--develop')
executed, filename = _version.main(False, args=args)
Copy link
Member

Choose a reason for hiding this comment

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

I would recommend to only pass boolean args by keyword.

Copy link
Member Author

Choose a reason for hiding this comment

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

The called main looks like this:
def main(leave=True, args=None):
So i can not pass any parameter but leave and args...

Copy link
Member

Choose a reason for hiding this comment

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

For the readability it is always better to pass pass booleans as keywords. somefunc(True, False, True) is not readable. In our case version.main(leave=False, args=args) is easier to read.

Co-authored-by: Björn Ricks <bjoern.ricks@greenbone.net>
@y0urself y0urself merged commit 8a65fe2 into greenbone:master Jun 8, 2021
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.

None yet

2 participants