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

Make --version and --help commands work without a project #115

Open
wants to merge 3 commits into
base: cudos-dev
Choose a base branch
from

Conversation

diogofferreira
Copy link

@diogofferreira diogofferreira commented Oct 24, 2022

Make --version and --help commands work without forcing the user to create a project or being inside a project folder.

Asana ticket: https://app.asana.com/0/1199899727964535/1203215140408171

Copy link

@nbaum nbaum left a comment

Choose a reason for hiding this comment

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

This PR does what it says, but it could be a little bit simpler and still do the same thing.

@@ -9,37 +9,42 @@ const { customTasks } = require('../utilities/task')
const { getConfig } = require('../utilities/config-utils')

async function main() {
if (hideBin(process.argv)[0] !== 'init') {
getConfig()
Copy link

Choose a reason for hiding this comment

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

Actually, removing the getConfig alone is sufficient! Any command which requires blast.config.js also calls getConfig itself. (getConfig doesn't store the config it loads anywhere, it just returns it. So anything which actually wants the config must call it again.)

It looks like getConfig is only called here to make blast fail immediately if there isn't a config, but it will always fail later if a command tries to actually use the config.

As it stands, putting getConfig inside a try block is actually the same as not doing it, because the only effect getConfig has here is to throw an immediately ignored exception if the config isn't there, and do nothing if it is.

Copy link
Author

Choose a reason for hiding this comment

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

That is true for everything but the custom task command. Custom tasks were implemented in a different way from all other commands, with no entry points nor config loading (even though it needs it). The getConfig on main was in fact the only way custom tasks were getting the config, and simply transferring it to the task class won't work (at least with no exception handling or refactoring), as it will trigger a "config file now found" exception when out of a project folder.

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