Skip to content

Conversation

@letitz
Copy link
Collaborator

@letitz letitz commented Jan 27, 2026

Opening on behalf of @mverde to start reviewing early.

Copy link
Collaborator Author

@letitz letitz left a comment

Choose a reason for hiding this comment

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

First round. I'm excited to see this land!

Copy link
Collaborator Author

@letitz letitz left a comment

Choose a reason for hiding this comment

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

Smaller comments this time around, except that we still need tests :)

to_extract = [f.name for f in to_extract]
self.assertCountEqual(to_extract, needed_files)

@parameterized.parameterized.expand(['/b/build/', 'build/', ''])
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why delete this test?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah sorry, this is actually something I meant to ask about but forgot over the weekend... this test seems exactly the same as the test that proceeds it other than in name. Do you happen to know if the test case is still relevant?

@cemon721-a11y
Copy link

cemon721-a11y commented Jan 30, 2026 via email

Copy link
Collaborator Author

@letitz letitz left a comment

Choose a reason for hiding this comment

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

Nearly there!

"""
self._set_archive_schema_version(1)
deps_entries = self._generate_possible_fuzzer_dependencies('my_fuzzer')
deps_files = self._generate_normalized_dependency_filenames(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This function calls _generate_possible_fuzzer_dependencies() again even though we just did on the line above. How about passing in a list of relative paths directly, and calling this something like _resolve_relative_dependency_paths(fuzz_target_path, dependency_paths)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I did something similar... how does this look?

Copy link

@cemon721-a11y cemon721-a11y left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix! Using getattr() to handle the missing attribute looks like the right approach. This will unblock our workflow." And if I did again some mistakes the kindly please Send Simply inbox To my Gmail address.

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.

3 participants