Skip to content

Conversation

@McGiverGim
Copy link
Member

Adds the Vue devtools to the yarn start command. It does not add it to other "similar" commands like gulp debug, maybe we can look to do the environment injection in the gulp file and not in the package to achieve the same.

This PR uses cross-env to fix the problem of environment variable in windows, but maybe is better to use this other solution: #2226 based on script-os. So first we need to decide which of them we take before merging this.

@mikeller
Copy link
Member

To be fair I prefer script-os, as it works at a lower level (i.e. it can be used to make things in yarn targets other than environment variables platform dependent.

@mikeller mikeller added this to the 10.8.0 milestone Oct 15, 2020
mikeller
mikeller previously approved these changes Oct 15, 2020
Copy link
Member

@mikeller mikeller left a comment

Choose a reason for hiding this comment

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

I think making this work for the yarn... invocation is entirely sufficient - this is the correct way to invoke gulp, as calling gulp... directly will call the gulp binary in the path, which may or may not end up using the version of gulp that is specified in package.json.

@McGiverGim
Copy link
Member Author

To be fair I prefer script-os, as it works at a lower level (i.e. it can be used to make things in yarn targets other than environment variables platform dependent.

Perfect, I will rebase this after the other PR is merged.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.8% 0.8% Duplication

@McGiverGim
Copy link
Member Author

Rebased and ready to be merged!!!

@mikeller mikeller merged commit 17972e5 into betaflight:master Oct 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants