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.

Sign up for GitHub

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

Jump to bottom

Fix asset loader for sourceassets #26636

Merged
merged 1 commit into from
Dec 20, 2024
Merged

Fix asset loader for sourceassets #26636

merged 1 commit into from
Dec 20, 2024
+7 −7

Conversation

dpeng817
Copy link
Contributor

@dpeng817 dpeng817 commented Dec 20, 2024 •
edited
Loading

Summary & Motivation

Fix some goofiness regarding source asset group_name remapping;

  • If you try to remap the group name to have the same group name, then allow that.
  • In the error message, tell us what the original group_name is and what we're trying to change it to.
  • We were hitting an error in DOP because we were trying to set group_name back to itself, which is not currently allowed. So fix that as well (although that is now allowed).

How I Tested These Changes

Existing tests

@dpeng817 Graphite App
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@dpeng817 dpeng817 requested a review from OwenKephart December 20, 2024 16:17
@dpeng817 dpeng817 marked this pull request as ready for review December 20, 2024 16:17
@dpeng817 dpeng817 force-pushed the dpeng817/fix_loader branch from 32afa63 to 7764d6d Compare December 20, 2024 16:18
@dpeng817 Graphite App
Copy link
Contributor Author

dpeng817 commented Dec 20, 2024 •
edited
Loading

Merge activity

  • Dec 20, 11:39 AM PST: A user started a stack merge that includes this pull request via Graphite.
  • Dec 20, 11:40 AM PST: A user merged this pull request with Graphite.

@dpeng817 dpeng817 merged commit 89a75eb into master Dec 20, 2024
1 check passed
@dpeng817 dpeng817 deleted the dpeng817/fix_loader branch December 20, 2024 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OwenKephart OwenKephart OwenKephart approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dpeng817 @OwenKephart