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

bw metadata: Don’t resolve Faults by default #758

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

vain
Copy link
Member

@vain vain commented May 21, 2024

This implements #557.

@@ -117,6 +117,15 @@ def __init__(self, fault_identifier, callback, **kwargs):
self.callback = callback
self.kwargs = kwargs

def _repr_first(self):
if isinstance(self.id_list, list):
return f"<Fault: {self.id_list[0]}>"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sold on this. If a secret is formatted into something else, i would like to know that. On the other hand, id_list might get very long, and we don't want to clutter the whole screen?

Copy link
Member Author

@vain vain May 21, 2024

Choose a reason for hiding this comment

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

It can get very long, see reasoning here: https://github.com/bundlewrap/bundlewrap/pull/758/files#diff-58547683697529b0873aa6b387d16f0fefc4e1430e101f8d54198336b62a7b31R305-R312

The usual __repr__() does this:

image

While _repr_first() does this:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

… I just had a use case where the full list would have been helpful. Need to think about this.

--unresolved-faults-full would do the trick, but feels quite clunky. 🫤

Copy link
Member Author

Choose a reason for hiding this comment

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

I suggest merging this as is. --unresolved-faults-full is probably very rare. Let’s see how often we actually need that.

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.

None yet

2 participants