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

Add option "required" to builder configs which will cause rosdoc_lite to fail if the builder fails. #106

Merged
merged 4 commits into from Jun 23, 2023

Conversation

peci1
Copy link
Contributor

@peci1 peci1 commented May 1, 2023

Resolves #63.

Each builder gets an optional parameter required. If set to True, rosdoc_lite will fail if the builder failed. However, all other builders are given the option to finish their run before rosdoc_lite is exited, so that as much documentation is generated as possible.

I've set the parameter to default to False so that the feature is opt-in. This should leave all existing packages without a change, but new packages are allowed to opt in. We could also change the documentation to consistently add required: True in the examples, with the note that it can be turned off.

@tfoote Would this be a good approach?

Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

Overall this looks like a good way to enable this.

I had one code comment. The documentation for generate_docs should be extended to document the potential exception in the doc block.

And I'd like to see a little bit more added to the documentation to document this capability. But overall this looks like a great addition.

src/rosdoc_lite/__init__.py Outdated Show resolved Hide resolved
peci1 and others added 2 commits May 2, 2023 01:20
@peci1
Copy link
Contributor Author

peci1 commented May 1, 2023

Thanks for the review. I've updated the code docs a bit. However, the main documentation is on the Wiki, which I'll update when this PR is merged.

Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

This looks good to me. Thanks for the iteration.

@tfoote tfoote merged commit 5338b2c into ros-infrastructure:master Jun 23, 2023
@peci1
Copy link
Contributor Author

peci1 commented Jun 25, 2023

With no outsanding PRs, would it be a good time to make a release?

@peci1
Copy link
Contributor Author

peci1 commented Jun 25, 2023

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.

No return code indicating failure
2 participants