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

feat: Don't proactively create the legacy fvm_config.json file #668

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mrgnhnt96
Copy link
Contributor

I found that when I ran fvm use stable the legacy fvm_config.json was created.

This PR adds a check for if the legacy config file exists, then it can write to it. If it doesn't, it get skipped.

There was also a redundant check that I removed. Calling file.write(...) will already check if the file exists and to create it if it doesn't.

Copy link

vercel bot commented Feb 28, 2024

@mrgnhnt96 is attempting to deploy a commit to the FlutterTools Team on Vercel.

A member of the Team first needs to authorize it.

@leoafarias
Copy link
Owner

@mrgnhnt96 As much as I agree 100% with this PR, I can't bring it in as is.. The issue is that there is still CI/CD, that is dependent on this file, and the .fvm folder is ignored, so this file will not exist.

Question: Does it matter to you if the file is automatically generated if the .fvm directory is ignored on .gitignore?

I think a solution would be to offer a "legacyConfig" flag on the project configuration, which allows for support of this fvm_config.json, or also do some type of CI/CD env var detection to create it.

@mrgnhnt96
Copy link
Contributor Author

Hey @leoafarias!

What exactly does the CI/CD depend on? Is it the flutter version?

I don't really care if it is created since the directory is not checked to VC. It just seemed like a bug since fvm_config.dart is now considered as legacy.

@leoafarias
Copy link
Owner

Hey @leoafarias!

What exactly does the CI/CD depend on? Is it the flutter version?

I don't really care if it is created since the directory is not checked to VC. It just seemed like a bug since fvm_config.dart is now considered as legacy.

This is on FVM's end, I am trying to figure out what could be causing this.

@mrgnhnt96
Copy link
Contributor Author

Happy to take a look for you too if you need, just lmk

@leoafarias
Copy link
Owner

Happy to take a look for you too if you need, just lmk

Its solved for now...

@leoafarias
Copy link
Owner

@mrgnhnt96, I will leave this open for now. We might have to put the legacy configuration generation behind a flag. But I am not 100% sure yet.

@mrgnhnt96
Copy link
Contributor Author

I will leave this open for now. We might have to put the legacy configuration generation behind a flag

Not a problem! Let me know if you have anything you'd like for me to help with

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