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

[3.x] Fix Story & Task option merge priority #249

Closed
wants to merge 1 commit into from

Conversation

Tofandel
Copy link

@Tofandel Tofandel commented Apr 26, 2022

See #248

Consider the following

@servers(['local' => 'localhost', 'dev' => 'someserver'])

@story('deploy', ['on' => 'dev'])
task_1
task_2
@endstory

@task('task_1')
echo 'task_1';
@endtask

@task('task_2', ['on' => 'local'])
echo 'task_2';
@endtask

Currently task_2 will be executed on the remote server instead of the expected localhost because the story options have priority over the task options but the behavior should be the opposite because a task is more specific than a story

After this PR the task options will correctly override the story options and task_1 will be executed on dev and task_2 on local

@taylorotwell
Copy link
Member

I don't plan to make any breaking changes to this library since there is an easy work around.

@Tofandel
Copy link
Author

Tofandel commented Apr 26, 2022

@taylorotwell this fix is required though because first it is not that easy to do when you have 20 tasks on a file and second the workaround is not applicable when using file inheritance between projects (which I'm doing) so it's becoming a big problem (eg story in one file extending tasks in another file)

@Tofandel
Copy link
Author

There is the possibility to add an option to story that would change the inheritance priority that would avoid a BC break

@driesvints
Copy link
Member

We already tried this. See #157 and then had to revert it because of #186. We probably will never change this sorry.

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