Skip to content

Conversation

glandium
Copy link
Contributor

This is especially useful when some of the kinds the caller is interested in is a dependency of another kind.

@glandium glandium requested a review from a team as a code owner August 20, 2025 05:50
@glandium glandium requested a review from bhearsum August 20, 2025 05:50
@glandium glandium force-pushed the load_tasks_for_kinds branch from b1a20b9 to 5e8e72f Compare August 20, 2025 06:31
Copy link
Contributor

@jcristau jcristau left a comment

Choose a reason for hiding this comment

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

Would you mind adding a test for the new function?

"""
Get all the tasks of a given kind.
Get all the tasks of a the given kinds.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Get all the tasks of a the given kinds.
Get all the tasks of the given kinds.

@glandium
Copy link
Contributor Author

glandium commented Aug 20, 2025

Would you mind adding a test for the new function?

Adding tests unveils that the fake_loader here sets tasks.metadata.name to the name of the task without the kind, while the taskgraph on firefox-main would use the same as the label, so including the kind. Is that a purposeful difference? Because of course, without the kind, names are overlapping, so the result from load_tasks_for_kinds is incomplete.
But then maybe load_tasks_for_kinds should be changed to use the label instead of metadata.name?

@jcristau
Copy link
Contributor

There seem to be other places where we assume the name matches the label e.g.

label = task["metadata"]["name"]
; so changing the test fixture to set name to the same thing as label seems like a reasonable change, and would match
"name": task["label"],

This is especially useful when some of the kinds the caller is
interested in is a dependency of another kind.
@glandium glandium force-pushed the load_tasks_for_kinds branch from 5e8e72f to 7ce050c Compare August 21, 2025 08:49
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.

2 participants