Allow the GH issue + Section as --flags and take news via stdin #45
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit adds three new options to 'blurb add' for automation: - --gh_issue: Specify GitHub issue number - --section: Specify NEWS section (must be from the sacred list) - --rst_on_stdin: Read the news entry from stdin (no editor needed) When using --rst_on_stdin, one must provide both --gh_issue and --section, lest they be turned into a newt (they'll get better). Added comprehensive test coverage including: - Unit tests for parameter validation - Integration tests with mock CPython repo (ruled by Brian of Nazareth) - CLI tests that actually run the blurb command Also fixed the command-line parser to handle non-boolean options specifically for the add command, and improved error handling for temporary file cleanup. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
This holy grail of refactoring reduces the code by >150 lines! We've eliminated the bureaucracy of wrapper functions and now apply `@app.command` decorators directly to the original functions. Just like the Knights of Ni demanded a shrubbery, cyclopts demands clear, typed parameters - and we have delivered! The code is now cleaner than the Holy Grail's cup after being washed by the French Taunter. Key improvements: - Moved cyclopts imports to top of file (no more silly walks) - Applied `@app.command` directly to functions (no indirection!) - Removed 'help' parameter from add() - cyclopts handles it - Simplified main() from ~200 lines to ~40 lines - Updated tests for new option naming (--gh-issue not --gh_issue) - All 28 tests still pass - it's not dead yet! 'Tis but a scratch compared to the original implementation! 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> (with a few human touchups by Greg)
gpshead
force-pushed
the
knights-who-say-stdin
branch
from
4249861 to
67a6c0c
Compare
gpshead
force-pushed
the
knights-who-say-stdin
branch
from
56b3eab to
e79dd63
Compare
gpshead
changed the title
|
May I suggest keeping the cycleopts stuff (looks better than our ugly hacks) but keep my way for rendering available sections etc? (because I also support URLs extractions which are usually easier when coming from a browser) |
Integrate the user-friendly features from PR python#16 by @picnixz into the automation support from PR python#45, making the CLI more intuitive: - Change --gh-issue to --issue, accepting multiple formats: * Plain numbers: --issue 12345 * With gh- prefix: --issue gh-12345 * GitHub URLs: --issue python/cpython#12345 - Add smart section matching with: * Case-insensitive matching: --section lib matches "Library" * Partial matching: --section doc matches "Documentation" * Common aliases: --section api matches "C API" * Separator normalization: --section core-and-builtins - Improve error messages for invalid sections This combines the automation features from PR python#45 with the interface improvements suggested by @picnixz in PR python#16, as reviewed by @hugovk and @larryhastings. Co-authored-by: picnixz <[email protected]>
Integrate the user-friendly features from PR python#16 by @picnixz into the automation support from PR python#45, making the CLI more intuitive: - Change --gh-issue to --issue, accepting multiple formats: * Plain numbers: --issue 12345 * With gh- prefix: --issue gh-12345 * GitHub URLs: --issue python/cpython#12345 - Add smart section matching with: * Case-insensitive matching: --section lib matches "Library" * Partial matching: --section doc matches "Documentation" * Common aliases: --section api matches "C API" * Separator normalization: --section core-and-builtins - Improve error messages for invalid sections This combines the automation features from PR python#45 with the interface improvements suggested by @picnixz in PR python#16, as reviewed by @hugovk and @larryhastings.
Now 'blurb -i 123' works the same as 'blurb add -i 123', because nobody expects the Spanish Inquisition... or having to type 'add' every time. - Forward all add command parameters to the default command - Update help text to indicate 'add' is the default command - Parameters like --issue, --section, and --rst-on-stdin now work directly with 'blurb' without specifying 'add'
|
I brought in the #16 features, please take another look. |
Eliminated 4 redundant test methods from TestAddCommandAutomation that duplicated validation logic already covered by existing parametrized tests. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
picnixz
reviewed
Jul 4, 2025
| normalized = ''.join(section_words).lower() | ||
|
|
||
| # Check special aliases | ||
| aliases = { |
There was a problem hiding this comment.
Maybe make this a global variable?
| GitHub issue number or URL (e.g. '12345', 'gh-12345', or 'https://github.com/python/cpython/issues/12345'). | ||
| section : str, optional | ||
| NEWS section. Can use partial matching (e.g. 'lib' for 'Library'). One of {sections_csv}. | ||
| rst_on_stdin : bool |
There was a problem hiding this comment.
Can we have a short option for this one, maybe -D for description for instance?
| @@ -57,13 +53,19 @@ | |||
| import tempfile | |||
| import textwrap | |||
| import time | |||
| from typing import Optional, Annotated | |||
There was a problem hiding this comment.
Suggested change
| from typing import Optional, Annotated | |
| from typing import Annotated, Optional |
Can we add a comment saying that it's needed for cycleopts? Otherwise this could be assumed to be unnecessary.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
At a high level I wanted this feature:
blurb addcommand:-i--issueoption to specify GitHub issue number-s--sectionoption to specify NEWS section-i/--issueand-s/--sectionflags toblurb add#16.--rst-on-stdinoption to read entry content from stdincycloptsfor command line parsing instead of rolling our own to reduce our code size, this changes the help format and brings in a dependency.That led to a rabbit hole, I did the feature in the first commit without cyclopts and really wished we were using it (or the older defopt) given the decorated function pattern being used for commands already.
More code deleted than added while gaining the above features. (until pulling in #16 brought in more, but that makes things nicer anyways)