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

feat: Support removing unused services only in dev environment #2581

Merged

Conversation

kayw-geek
Copy link
Contributor

Before people always to removing those unused services in any environments, If they miss some services that they are still using, that will cause some risks.

Now you can use the onlyRemoveUnusedServicesInDev method to avoid the risks.

@kayw-geek kayw-geek changed the title Support removing unused services only in dev environment feat: Support removing unused services only in dev environment Nov 24, 2022
@kayw-geek
Copy link
Contributor Author

@stobrien89 Could you help me to review the PR?

@kayw-geek
Copy link
Contributor Author

@stobrien89 Have you any time help me to review the PR?

@stobrien89
Copy link
Member

Hi @kayw-geek,

Sorry for the delay. Haven't forgotten about this— We have a backlog of higher priority PRs we'll need to merge first, but we plan to focus more on community contributions in the coming weeks. I'll let you know when I have a chance to review!

@kayw-geek
Copy link
Contributor Author

Hi @kayw-geek,

Sorry for the delay. Haven't forgotten about this— We have a backlog of higher priority PRs we'll need to merge first, but we plan to focus more on community contributions in the coming weeks. I'll let you know when I have a chance to review!

Sure, never mind, it is not a important PR, thank your help me.

@kayw-geek
Copy link
Contributor Author

@

Hi @kayw-geek,

Sorry for the delay. Haven't forgotten about this— We have a backlog of higher priority PRs we'll need to merge first, but we plan to focus more on community contributions in the coming weeks. I'll let you know when I have a chance to review!

Hi @stobrien89 You got a minute?
I am very much looking forward to this improvement.

Copy link
Member

@stobrien89 stobrien89 left a comment

Choose a reason for hiding this comment

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

Hi @kayw-geek,

Sorry again for the delay. This looks good for the most part, just had a nit and question about backward-compatibility. We're deprecating PHP < 7.2.5 in a few months, so we might want to wait until that happens to be sure there won't be any issues.

src/Script/Composer/Composer.php Outdated Show resolved Hide resolved
src/Script/Composer/Composer.php Outdated Show resolved Hide resolved
@kayw-geek kayw-geek force-pushed the feature/add-remove-unused-services-in-dev branch 2 times, most recently from 9a606fa to c2e5220 Compare March 20, 2023 02:03
@kayw-geek kayw-geek requested a review from stobrien89 March 20, 2023 02:04
@kayw-geek
Copy link
Contributor Author

kayw-geek commented Mar 24, 2023

Hi @stobrien89, I hope you're having a great day. I wanted to follow up on my PR and see if you had any feedback or suggestions. If you need any additional information or have any questions, please don't hesitate to ask. Thank you for your time and effort.

@kayw-geek
Copy link
Contributor Author

@stobrien89 Could you please review this again thanks

src/Script/Composer/Composer.php Outdated Show resolved Hide resolved
src/Script/Composer/Composer.php Outdated Show resolved Hide resolved
src/Script/Composer/README.md Outdated Show resolved Hide resolved
@kayw-geek kayw-geek force-pushed the feature/add-remove-unused-services-in-dev branch from c2e5220 to 2fc8304 Compare June 4, 2024 01:44
@kayw-geek
Copy link
Contributor Author

Hi @yenfryherrerafeliz, I have updated these code, could you please review this again thanks.

@stobrien89
Copy link
Member

Thanks @kayw-geek!

Apologies for the delay on this.

@stobrien89 stobrien89 merged commit ea6cd3f into aws:master Jun 17, 2024
18 checks passed
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