-
Notifications
You must be signed in to change notification settings - Fork 18
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
Update newdoc: Fix the optional formatting and drop Python2 #30
Conversation
Created by command: /usr/bin/tito tag --use-version=1.5.0 --use-release=1
I'm going to leave this PR open for a couple of days to see if anybody has any comments. Then I'll merge it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thank you, and happy releasing!
.tito/packages/python-newdoc
Outdated
@@ -1 +1 @@ | |||
1.4.3-1 newdoc/ | |||
1.5.0-1 newdoc/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a thought: What are the costs of using a Makefile or m4 or something to have this is one place? I see the same number in at least 3 places...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Fortunately, the tito
build tool mostly takes care of this: I need to set the version manually in the newdoc
command-line parser, and then tag a new version with tito
, which updates the version in all the remaining places.
Still, I've forgotten to update the command-line parser a couple of times already.
if PYVERSION == 2: | ||
reload(sys) | ||
sys.setdefaultencoding('utf8') | ||
import configparser as cp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not part of the changes, but you could easily import the three things instead of the module, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean ConfigParser
, MissingSectionHeaderError
, and NoOptionError
? Sure, importing them directly could somewhat optimize the performance if I called the objects in a loop. But since they aren't in a performance-sensitive place, I prefer to keep the namespace to make it clear where the objects come from.
for k in options.keys(): | ||
# The configparser library is different in Python 2 and 3. | ||
# This si the only 2/3-compatible way I've found for optional keys. | ||
# This is the only 2/3-compatible way I've found for optional keys. | ||
try: | ||
options[k] = config.get("newdoc", k) | ||
except cp.NoOptionError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you want to somehow resolve this too? The comment seems to indicate you could do this "better". But then again, the code already works, so it does not matter much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're absolutely right. It could be improved, but there's no reason to do it since the current code works.
@VladimirSlavik, thanks for the review. It means even more now that you're on the other side of needinfo 🙂 By the way, I've been rewriting the project to Rust because I wanted to learn the language. Turns out, I'm happier with the end result, it has more features now, and I'll probably deprecate this Python version in favor of the Rust rewrite. In case you want to look around: |
Oh! Delightful. It will be still newdoc? It's missing a R! |
Created by command: /usr/bin/tito tag --use-version=1.5.1
Yes, it's still called
I looked for a way to add the Rust "R" in a project but didn't find anything. Do you know how to do it? |
I've just added another commit that deprecates this version of I'm going to merge everything now and I'm saying this just for the record. |
This PR updates newdoc to version 1.5.0, which includes the following changes:
The optional step in the procedure template has changed from
(Optional)
toOptional:
to match the IBM Style Guide. See Optional step #29.Python 2 support has been removed, following the development in Fedora and RHEL 8.
The
--version
option now reports the correct newdoc version.