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

Optional Mutagen #842

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Conversation

ihor-sviziev
Copy link
Contributor

Check List

  • Is there an existing issue that covers this? (link it here, if so)
  • Is there an entry in the CHANGELOG.md file?

Describe the bug
This is a follow-up on #749
To Reproduce
Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.

Check List

  • Matching PR in the documentation repo (replace text with link when it exists)
  • Entry in CHANGELOG.md

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

Describe the solution you've submitted
A clear and concise description of what you want to happen.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

jose luis munoz and others added 14 commits February 15, 2024 12:20
Make mutagen enabled by default only for Mac OS
Load only mutagen enable flag
Load only mutagen enable flag
Check only mutagen enable flag
Fail sync command when mutagen is not enabled
Simplify appendEnvPartialIfExists
Show warning only when using OrbStack
Rename files not related to mutagen
Rename mutagen docker compose files
Move warning to warden sync command
@ihor-sviziev
Copy link
Contributor Author

ihor-sviziev commented Feb 7, 2025

Hi @joseluisi4, @bap14,
I fixed all the issues listed in the pull request: #749.
However, I tested disabled mutagen with OMutagen on a quite big project (about 650 packages in composer)—the performance was almost the same but in some scenarios it was significantly slower than with Mutagen.

My measurements:

  • Response time of frontend and admin - about 10% slower w/o Mutagen
  • time (php bin/magento setup:upgrade) - about the same performance
  • time (rm -Rf ./vendor/ && composer i) - about 50% slower w/o Mutagen
  • time php bin/magento setup:di:compile - about 50% slower w/o Mutagen

So, I'm not sure if it's a good option to show a warning.

For myself, I decided that it's better to keep using Mutagen sync.

@ihor-sviziev ihor-sviziev marked this pull request as ready for review February 7, 2025 14:13
Do not show recommendation to disable Mutagen
@ihor-sviziev ihor-sviziev mentioned this pull request Feb 7, 2025
@diazwatson
Copy link

I think it is still good to give the option to devs

Copy link
Member

@navarr navarr left a comment

Choose a reason for hiding this comment

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

This looks good! I'll just need to dogfood it for a bit before merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants