fvm icon indicating copy to clipboard operation
fvm copied to clipboard

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

Open mrgnhnt96 opened this issue 4 months ago • 8 comments

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.

mrgnhnt96 avatar Feb 28 '24 20:02 mrgnhnt96

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

A member of the Team first needs to authorize it.

vercel[bot] avatar Feb 28 '24 20:02 vercel[bot]

@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.

leoafarias avatar Feb 28 '24 20:02 leoafarias

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.

mrgnhnt96 avatar Feb 28 '24 21:02 mrgnhnt96

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.

leoafarias avatar Feb 28 '24 21:02 leoafarias

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

mrgnhnt96 avatar Feb 28 '24 21:02 mrgnhnt96

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

Its solved for now...

leoafarias avatar Feb 28 '24 22:02 leoafarias

@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.

leoafarias avatar Feb 29 '24 13:02 leoafarias

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

mrgnhnt96 avatar Feb 29 '24 15:02 mrgnhnt96