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

refactor: use a metadata file for forges #1909

Merged
merged 4 commits into from
Apr 20, 2024
Merged

Conversation

roele
Copy link
Contributor

@roele roele commented Apr 16, 2024

This is a follow up on #1905 attempting to move the "un-dirname" logic out of the ForgeArg into the Forge implementations.

Additionally it seems for Go while package names cannot have -, module names can and often do. So the logic for Go will also not work reliably. For example https://pkg.go.dev/search?q=http reveals all sorts of exceptions.

Using a + as separator would yield better results but i'm afraid thats not easy to change now is it?

@roele roele force-pushed the issues/1905-refactor branch 3 times, most recently from b2e96c3 to 84b4854 Compare April 16, 2024 12:08
@jdx
Copy link
Owner

jdx commented Apr 16, 2024

I think we need to modify our strategy. Maybe we need a metadata file to store the real name

@roele
Copy link
Contributor Author

roele commented Apr 16, 2024

Agreed, the path is not suitable and will most likely continue to cause problems no matter what separator we use.

@jdx
Copy link
Owner

jdx commented Apr 16, 2024

I think it's worth doing though. For example, I bet mise upgrade probably needs this in order to work. I'm not sure if I've tested that with these yet or not.

@roele
Copy link
Contributor Author

roele commented Apr 16, 2024

Gave it a first shot but i'm not sure if we should use a structured file instead? Also the upgrade does not seem to work for all backends but i'm not sure if this is related or is caused by something else yet.

@roele roele changed the title refactor: move forge (un-)dirname logic to forge implementations refactor: use a metadata file for forges Apr 17, 2024
@roele roele marked this pull request as ready for review April 17, 2024 16:08
src/forge/forge_meta.rs Outdated Show resolved Hide resolved
@jdx jdx merged commit 2ded275 into jdx:main Apr 20, 2024
7 checks passed
@roele roele deleted the issues/1905-refactor branch April 20, 2024 21:04
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