8000 refactor: unify content of ActionCoords name by Vampire · Pull Request #1646 · typesafegithub/github-workflows-kt · GitHub
[go: up one dir, main page]

Skip to content

refactor: unify content of ActionCoords name #1646

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

Merged
merged 1 commit into from
Oct 27, 2024

Conversation

Vampire
Copy link
Collaborator
@Vampire Vampire commented Sep 8, 2024

Before these changes, ActionCoords#name sometimes had the actual action coordinates, so for example some-name__some-sub, sometimes it had the replaced some-name/some-sub.
This is not good, as you never know what you get and also led to certain bugs where wrong things are written to metadata files for example.
This changes unifies the situation, so that the name only contains the repository name and the path is in a separate property with slashes.
All other manipulation is then done where needed.

Copy link
Collaborator Author
Vampire commented Sep 8, 2024

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

@Vampire Vampire force-pushed the vampire/unify-coords-name branch 7 times, most recently from 02098ce to 37375d9 Compare September 10, 2024 16:42
Copy link
Member
@krzema12 krzema12 left a comment

Choose a reason for hiding this comment

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

There's a lot going on here - I presume that you have a certain principal idea in your mind. Could you share more context in PR description?

@Vampire
Copy link
Collaborator Author
Vampire commented Sep 23, 2024

Done

@Vampire Vampire requested a review from krzema12 September 23, 2024 08:13
@Vampire Vampire force-pushed the vampire/unify-coords-name branch 2 times, most recently from c1dc24a to 6d56a0d Compare September 24, 2024 00:42
@Vampire Vampire force-pushed the vampire/unify-coords-name branch from 37ad070 to 52919f6 Compare October 27, 2024 04:01
Copy link
Member
@krzema12 krzema12 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 unifying stuff and also simplifying some pieces!

@@ -287,6 +287,30 @@ class GenerationTest :
binding.shouldContainAndMatchFile("ActionWithNoInputs.kt")
}

test("subaction") {
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding this missing test 👍

@krzema12 krzema12 changed the title chore(abg): Unify content of ActionCoords name refactor: unify content of ActionCoords name Oct 27, 2024
@krzema12 krzema12 merged commit 0343909 into main Oct 27, 2024
18 checks passed
@krzema12 krzema12 deleted the vampire/unify-coords-name branch October 27, 2024 09:52
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
0