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

[FEATURE] Added Blackfire Support #504

Open
wants to merge 6 commits into
base: 2.x
Choose a base branch
from

Conversation

lewisvoncken
Copy link
Contributor

@lewisvoncken lewisvoncken commented Jun 18, 2020


I have read the contribution guidelines and am targeting the branch 2.x:
Because this is a Feature which is Backwards Compatible.

Changelog entry:
Short description of your work as explained by: Keep A Changelog.

Added

  • Blackfire Support wiki changes are added in a patch file

https://github.com/weprovide/valet-plus/files/4179355/blackfire_wiki.txt

@lewisvoncken lewisvoncken changed the title Patch 6 [FEATURE] Added Blackfire Support Jun 18, 2020
@Neodork
Copy link
Collaborator

Neodork commented Oct 3, 2020

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Neodork Neodork added 2.x Issues / features related the the 2.x branch MINOR This change is minor labels Oct 3, 2020
@Neodork
Copy link
Collaborator

Neodork commented Oct 3, 2020

@lewisvoncken All tests pass, can you create a issue ticket for this explaining why Valet+ would need this implemented? Edit: It also seems to conflict. Can you solve the conflicts as well?

@mischabraam
Copy link
Member

Tests my be passed, but I don't think this is complete. The switch case for commands has no breaks. So the command valet blackfire test will not break the switch untill valet blackfire disable|off. I don't think that is correct behavior. Could be, but I'd rather see a clarification first.

@SanderAtom
Copy link
Collaborator

SanderAtom commented Jan 11, 2021

@lewisvoncken Can you clarify why the test, register and config commands have no breaks/returns? If you run test, it will also run the through the register, config and off commands.
I'm going to apply this locally and run it because I need to use Blackfire, so I don't mind fixing that if it's a bug.

@SanderAtom
Copy link
Collaborator

The installation of Blackfire breaks if you don't have the right xcode version.

==> Installing blackfire-agent from blackfireio/blackfire
==> Downloading https://packages.blackfire.io/homebrew/blackfire-agent_1.45.1_amd64.tar.gz
Error: Your Xcode (10.2.1) is too outdated.
Please update to Xcode 12.3 (or delete it).

Can we check for this before installing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Issues / features related the the 2.x branch MINOR This change is minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants