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 skip_failed flag #121

Closed
wants to merge 2 commits into from
Closed

add skip_failed flag #121

wants to merge 2 commits into from

Conversation

FoamyGuy
Copy link
Contributor

@FoamyGuy FoamyGuy commented Sep 23, 2024

Resolves: #120

This adds a new --skip_failed flag to the CLI and relevant python functions, it defaults to False if unused so behavior stays the same as today.

If the new flag is passed it will continue the build process instead of bailing after a failed build within a library.

I will open a PR shortly in the community bundle that will utilize this flag to allow it build what it can even if some libraries do fail.

An open question that I'm interested in thoughts on is what to do in the case of the failed builds. It will still print the errors in the actions output, but with the Task passing it's unlikely to be seen there by anyone. I would guess without some other mechanism the most likely time it would be discovered is when someone tries to find that library within the bundle and is unable to.

Maybe a notice message or warning message

@@ -47,7 +47,7 @@ jobs:
# Use the community bundle because it's smaller and faster
git clone --recurse-submodules https://github.com/adafruit/CircuitPython_Community_Bundle.git
cd CircuitPython_Community_Bundle
circuitpython-build-bundles --filename_prefix test-bundle --library_location libraries --library_depth 2
circuitpython-build-bundles --filename_prefix test-bundle --library_location libraries --library_depth 2 --skip_failed
Copy link
Member

@jepler jepler Sep 24, 2024

Choose a reason for hiding this comment

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

I added more notes to the related issue but I want to note here that I'm uneasy about adding skip_failed during the test of circuitpython-build-tools: If a bug is introduced into the code here, --skip_failed might cover it up.

Instead, consider using the communty bundle at a specific ref/tag that is known to build properly, and remove --skip_failed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think about having this task use the main Adafruit Bundle instead of the community bundle?

The comment here notes that community bundle was chosen because it's smaller and thus faster. It is still true that the community bundle is smaller, but it's also gaining ground at ~158 libraries right now. I think the main bundle is ~340ish but am not certain. I imagine that the disparity was greater when this task was initially set up.

I am not so sure that the increased speed is that big of a benefit. As I understand it this is always run automatically inside of github actions, there isn't typically a person sitting and waiting for it to complete.

In my mind pointing it to the main Adafruit Bundle would also be nice because it will give another chance to discover ASAP if any changes to the build-tool could cause any issues for the Adafruit libraries or bundle.

@FoamyGuy
Copy link
Contributor Author

FoamyGuy commented Oct 7, 2024

Closing this in favor of: adafruit/adabot#378

I do still think it would make sense for the actions to test against the Adafruit Bundle instead of the community one. But that could be done separately and doesn't need this skip_failed concept to exist.

@FoamyGuy FoamyGuy closed this Oct 7, 2024
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.

More Forgiving Handling of Failed Build
2 participants