Skip to content

Allow disabling default build if multiple entry points are defined #37

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

Merged
merged 1 commit into from
Mar 29, 2019

Conversation

emodric
Copy link
Contributor

@emodric emodric commented Jan 28, 2019

Fixes #32.

@emodric emodric force-pushed the disable_default_entry branch from 191a60c to b6db7e1 Compare January 28, 2019 12:38
Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

Hey @emodric!

Sorry for the slow attention on this. I merged another PR and created conflicts with this one - could you check those?

I'm +1 for this PR. However, I'm interested in what the error is if you disable the default entrypoint, and then accidentally use a feature without specifying which build you want - I think the user will see errors like "The build _default does not exist" - which isn't a great error.

@weaverryan
Copy link
Member

#45 will also help and affect this PR

@emodric
Copy link
Contributor Author

emodric commented Mar 1, 2019

@weaverryan Will get back to you next week, can't get to it any sooner.

@emodric emodric force-pushed the disable_default_entry branch from b6db7e1 to dbdaaaa Compare March 11, 2019 13:32
@emodric
Copy link
Contributor Author

emodric commented Mar 11, 2019

@weaverryan I've pushed the rebased commit. As for #45, how will it help and affect this PR?

When someone uses a default build which has been disabled, the error message is: Given entry point "_default" is not configured.

Maybe we could try to determine which builds are configured and offer them as an alternative in the error message, e.g: The default build is not enabled. Use one of "frontend", "backend", "app1", "app2" builds.

@jRexhmati
Copy link

Hello @emodric do you plan to merge this pr anytime soon?

@emodric
Copy link
Contributor Author

emodric commented Mar 27, 2019

You should probably be asking @weaverryan that :)

@jRexhmati
Copy link

@emodric Thanks for the info :)
@weaverryan Do you plan to merge this soon? I can give you a hand ✋if you need

@weaverryan
Copy link
Member

Sorry about the slow merge on this - it's a really great PR. Even the README got updated! Thank you Edi!

@weaverryan weaverryan merged commit dbdaaaa into symfony:master Mar 29, 2019
weaverryan added a commit that referenced this pull request Mar 29, 2019
…e defined (emodric)

This PR was merged into the master branch.

Discussion
----------

Allow disabling default build if multiple entry points are defined

Fixes #32.

Commits
-------

dbdaaaa Allow disabling default build if multiple entry points are defined
@emodric
Copy link
Contributor Author

emodric commented Mar 29, 2019

@weaverryan Thanks for the merge :)

However, what about this part, that you were initially concerned about?

Maybe we could try to determine which builds are configured and offer them as an alternative in the error message, e.g: The default build is not enabled. Use one of "frontend", "backend", "app1", "app2" builds.

weaverryan added a commit that referenced this pull request Apr 10, 2019
…is enabled (emodric)

This PR was merged into the master branch.

Discussion
----------

Only add alias to EntrypointLookupInterface if default build is enabled

Caused by the combination of 64f1e94 and #37

The commit in question was added after the commits in my PR #37, so it slipped through the cracks.

Commits
-------

315955f Only add alias to EntrypointLookupInterface if default built is enabled
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