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.

Sign up for GitHub

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

Jump to bottom

ReST export plugin should accept --template option #2218

Merged
merged 3 commits into from
Jul 28, 2023
Merged

ReST export plugin should accept --template option #2218

merged 3 commits into from
Jul 28, 2023

Conversation

happz
Copy link
Collaborator

@happz happz commented Jul 25, 2023

The option was accepted by template plugin only, but it absolutely makes sense for -h rst to accept it as well. There is a default story template, and no other template can be used...

@happz happz added bug Something isn't working command | export The export command labels Jul 25, 2023
@happz happz added this to the 1.26 milestone Jul 25, 2023
@happz happz requested review from psss, lukaszachy and thrix as code owners July 25, 2023 14:46
@happz happz changed the title ReST export plugin shoudl accept --template option ReST export plugin should accept --template option Jul 25, 2023
@happz happz force-pushed the rst-export-accept-template branch from 8be9a6b to 3b3ae1a Compare July 25, 2023 14:47
lukaszachy
lukaszachy approved these changes Jul 26, 2023
View reviewed changes
Copy link
Collaborator

@lukaszachy lukaszachy left a comment

Choose a reason for hiding this comment

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

I don't like the confusion with saying 'rst|template' only for Plans and Test exports as there is no 'rst' nor 'template' how. However we shouldnt have --template option there at all.

Thus it keeps current status quo.

@happz happz force-pushed the rst-export-accept-template branch from 3b3ae1a to eb2baec Compare July 26, 2023 15:02
@happz
Copy link
Collaborator Author

happz commented Jul 26, 2023

I don't like the confusion with saying 'rst|template' only for Plans and Test exports as there is no 'rst' nor 'template' how. However we shouldnt have --template option there at all.

Thus it keeps current status quo.

Good point, I didn't notice there's another bug hiding in plain sight, template export format was not enabled for all classes, just stories.

I tried to improve things in eb2baec: --how template should now be available for all exportable objects, and only tmt story export will mention rst|template in --template help text. Does this make things better?

@lukaszachy
Copy link
Collaborator

@happz Much better, thanks. Tests were expecting old help message thus fixed in 7de7b1f

LGTM

happz and others added 3 commits July 28, 2023 10:14
@happz @lukaszachy
ReST export plugin should accept --template option
0a681c6 
The option was accepted by `template` plugin only, but it absolutely
makes sense for `-h rst` to accept it as well. There is a default story
template, and no other template can be used...
@happz @lukaszachy
squash: register template for other classes too
70108ff
@lukaszachy
squash: fix tests
fe02bd5
@lukaszachy lukaszachy force-pushed the rst-export-accept-template branch from 7de7b1f to fe02bd5 Compare July 28, 2023 08:14
@lukaszachy lukaszachy added the status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. label Jul 28, 2023
@lukaszachy
Copy link
Collaborator

Rawhide failure in /plans/install/pip/full ::
ERROR: Could not build wheels for aiohttp, which is required to install pyproject.toml-based project

Problem already reported in aiohttp issue tracker - aio-libs/aiohttp#7229

IMO we have no other way than ignore this failure (what uses it btw?)

@lukaszachy lukaszachy merged commit 8f92f8a into main Jul 28, 2023
@lukaszachy lukaszachy deleted the rst-export-accept-template branch July 28, 2023 09:09
@psss
Copy link
Collaborator

psss commented Aug 8, 2023

IMO we have no other way than ignore this failure (what uses it btw?)

Seems this is pulled in by mrack:

Collecting aiohttp>=3 (from asyncopenstackclient>=0.8.1->mrack>=1.12.1->tmt==1.25.0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukaszachy lukaszachy lukaszachy approved these changes

@psss psss Awaiting requested review from psss psss is a code owner

@thrix thrix Awaiting requested review from thrix thrix is a code owner

Assignees
No one assigned
Labels
bug Something isn't working command | export The export command status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish.
Projects
None yet
Milestone
1.26
Development

Successfully merging this pull request may close these issues.
3 participants
@happz @lukaszachy @psss