CMakePP / CMinx

Generates API documentation for CMake functions and macros
https://cmakepp.github.io/CMinx/
Apache License 2.0
14 stars 5 forks source link

Clean up code #100

Closed AutonomicPerfectionist closed 2 years ago

AutonomicPerfectionist commented 2 years ago

Is this pull request associated with an issue(s)? Fixes #99

Description This PR adds inline comments and refactors code to be more idiomatic and pythonic.

TODOs

codecov[bot] commented 2 years ago

Codecov Report

Merging #100 (8c3f20c) into master (b61f682) will increase coverage by 0.01%. The diff coverage is 98.36%.

@@            Coverage Diff             @@
##           master     #100      +/-   ##
==========================================
+ Coverage   96.03%   96.05%   +0.01%     
==========================================
  Files           6        7       +1     
  Lines         807      836      +29     
==========================================
+ Hits          775      803      +28     
- Misses         32       33       +1     
Flag Coverage Δ
unittests 96.05% <98.36%> (+0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/cminx/__init__.py 93.19% <92.10%> (-0.05%) :arrow_down:
src/cminx/documentation_types.py 98.63% <98.63%> (ø)
src/cminx/aggregator.py 92.20% <100.00%> (ø)
src/cminx/config.py 100.00% <100.00%> (ø)
src/cminx/documenter.py 100.00% <100.00%> (+0.88%) :arrow_up:
src/cminx/rstwriter.py 98.24% <100.00%> (-0.02%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ryanmrichard commented 2 years ago

@AutonomicPerfectionist Unless you feel differently, I vote that merging this PR results in 1.0. Unfortunately, my guess is the PyPI upload of that 1.0 will fail because when I was messing around with PyPI packaging I pushed a release and tagged it as 1.0. I subsequently learned that PyPI is crazy strict with versioning and now that we've called something "CMinx 1.0", that's our 1.0 forever (FWIW I've now also learned that there's "Test PyPI" to practice deploying to PyPI). Anyways, long story short, once this PR gets merged we'll need to actually make a 1.1 release to get the PyPI releases going.

Edit: we may actually be okay because the version I had on PyPI was 1.0, and our git versions also include a patch; so I think this PR will actually result in a version 1.0.0

AutonomicPerfectionist commented 2 years ago

@ryanmrichard I agree that this PR should result in 1.0, as far as I'm aware there aren't any other outstanding issues other than #80. Does that one require the 1.0 release to be finalized before it can be fixed?

ryanmrichard commented 2 years ago

80 can be done after the 1.0. It's not very important.

On Fri, Aug 5, 2022, 6:35 PM Branden Butler @.***> wrote:

@ryanmrichard https://github.com/ryanmrichard I agree that this PR should result in 1.0, as far as I'm aware there aren't any other outstanding issues other than #80 https://github.com/CMakePP/CMinx/issues/80. Does that one require the 1.0 release to be finalized before it can be fixed?

— Reply to this email directly, view it on GitHub https://github.com/CMakePP/CMinx/pull/100#issuecomment-1207088658, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABIJDC3ISKKAY5U4WEFK6BLVXWQLJANCNFSM543EPNKQ . You are receiving this because you were mentioned.Message ID: @.***>

AutonomicPerfectionist commented 2 years ago

@ryanmrichard I think this one is ready for review now. Let me know if there's anything you want to be changed before we merge

ryanmrichard commented 2 years ago

🚀 [bumpr] Bumped! New version:v1.0.0 Changes:v0.1.4...v1.0.0