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

fix: make dev cmd consistent with deploy cmd #398

Merged
merged 1 commit into from
Nov 30, 2018

Conversation

thsig
Copy link
Collaborator

@thsig thsig commented Nov 30, 2018

Fixes #388.

Don't return a singleton build task by default in the dev command. This makes the dev command's behavior more consistent with the deploy command in watch mode.

@drubin
Copy link
Contributor

drubin commented Nov 30, 2018

Thanks for this I think this might solve some of the issues.

But it appears the structure/code is still slightly different between dev/deploy. My understanding is that the code should be fairly similar if not exactly the same and shareable but it's not.

I don't fully understand why? Why can't abstract it away so that when we fix/change something we change it in one place?

We have had a few bugs recently about the differences between these 2 commands behaving differently and every time we patch the logic.

Is there a solution of wrapping this login in a shared function that could potentially solve this once and for all?

@thsig
Copy link
Collaborator Author

thsig commented Nov 30, 2018

@drubin: True, the structure of dev vs deploy is still different in a couple of ways, mostly in that dev uses both the getTestTasks and getTasksForModule helpers to generate the task list in its handler and changeHandler, where deploy uses only getTasksForModule.

On the other hand, the test command only uses the getTestTasks handler, and the build command simply returns build tasks and doesn't use either of these helpers.

We could consolidate getTestTasks into getTasksForModule and add an includeTests param, but I thought it was clearer to have the commands mix and match them explicitly.

That said, getTasksForModule is a bad name, just couldn't think of anything better to describe its return value.

Fixes #388.

Don't return a singleton build task by default in the dev command. This
makes the dev command's behavior more consistent with the deploy command
in watch mode.
@thsig thsig force-pushed the dev-deploy-command-consistency branch from 237f1d6 to 85f31f9 Compare November 30, 2018 14:20
@thsig thsig merged commit 8bbc389 into master Nov 30, 2018
@thsig thsig deleted the dev-deploy-command-consistency branch November 30, 2018 14:27
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.

3 participants