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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable API by default in dev environment #12804

Merged
merged 1 commit into from Jul 16, 2021

Conversation

Zales0123
Copy link
Member

Q A
Branch? 1.9
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Related tickets
License MIT

I believe it would be useful for the development 馃枛 Without it if I want to change something in the API I need to change the parameter and pray to not forget to remove it before committing 馃檹 馃帀

@Zales0123 Zales0123 added Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). DX Issues and PRs aimed at improving Developer eXperience. labels Jul 15, 2021
@Zales0123 Zales0123 requested a review from a team as a code owner July 15, 2021 14:02
@Zales0123 Zales0123 added the API APIs related issues and PRs. label Jul 15, 2021
@GSadee
Copy link
Member

GSadee commented Jul 16, 2021

The base of this pull-request was changed, you need fetch and reset your local branch
if you want to add new commits to this pull request. Reset before you pull, else commits
may become messed-up.

Unless you added new commits (to this branch) locally that you did not push yet,
execute git fetch origin && git reset "enable-api-in-dev-by-default" to update your local branch.

Feel free to ask for assistance when you get stuck 馃憤

@GSadee GSadee changed the base branch from master to 1.9 July 16, 2021 09:18
@GSadee GSadee force-pushed the enable-api-in-dev-by-default branch from 89a6c9c to 0d53489 Compare July 16, 2021 09:18
@stloyd
Copy link
Contributor

stloyd commented Jul 16, 2021

In overall this is huge BC break to disabled in minor version something that was enabled before by default... cause as developer you don't expect to do update, have code working in dev env go to production & get fatal errors or 404s...

@GSadee GSadee merged commit 731ccdf into Sylius:1.9 Jul 16, 2021
@GSadee
Copy link
Member

GSadee commented Jul 16, 2021

Thank you, Mateusz! 馃

@Zales0123 Zales0123 deleted the enable-api-in-dev-by-default branch July 16, 2021 09:44
@Zales0123
Copy link
Member Author

@stloyd in general, you're totally right 馃枛 but because this new API is still in an experimental phase, a much bigger threat is to make it enabled by default and potentially introduce some security problems to the Sylius applications code 馃殌

GSadee added a commit to Sylius/Sylius-Standard that referenced this pull request Apr 13, 2022
This PR was merged into the 1.10 branch.

Discussion
----------

For easier development of testing of the new application 馃枛 It was already done for Sylius/Sylius [a long time ago](Sylius/Sylius#12804).

Commits
-------

634ecdf Enable Sylius API in dev by default
windragon0910 added a commit to windragon0910/symfony_ecom_framework that referenced this pull request Oct 25, 2023
This PR was merged into the 1.10 branch.

Discussion
----------

For easier development of testing of the new application 馃枛 It was already done for Sylius/Sylius [a long time ago](Sylius/Sylius#12804).

Commits
-------

634ecdf858a68c5bbecf53eff79272e78707746e Enable Sylius API in dev by default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API APIs related issues and PRs. DX Issues and PRs aimed at improving Developer eXperience. Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants