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

Allow crash reporters registration during app bootstrap #21457

Merged
merged 1 commit into from
Jun 19, 2020

Conversation

ChristophWurst
Copy link
Member

Before

This changes the crash reporters registration from

  • Create a crash reporter instance
  • Query the registry
  • Pass the object to the registry

… when we might not even use it.

After

It now just appends a string to an array. The service is only built if we really need it. Thus 🐨:rocket:

\OCP\Support\CrashReport\IRegistry is obsolete, so I've deprecated it.


For ChristophWurst/nextcloud_sentry#207, but needs #21456 to work properly.

@ChristophWurst ChristophWurst added enhancement 3. to review Waiting for reviews pending documentation This pull request needs an associated documentation update labels Jun 17, 2020
@ChristophWurst ChristophWurst added this to the Nextcloud 20 milestone Jun 17, 2020
@ChristophWurst ChristophWurst self-assigned this Jun 17, 2020
@rullzer
Copy link
Member

rullzer commented Jun 17, 2020

Thus 🐨 🚀

I don't know what this means. But I'm on board!

@ChristophWurst
Copy link
Member Author

ChristophWurst commented Jun 17, 2020

I don't know what this means. But I'm on board!

It's a very lazy but also very fast koala

@juliusknorr
Copy link
Member

php-cs is failing

@ChristophWurst ChristophWurst force-pushed the enhancement/crash-reporters-registration-boostrap branch from e0efd07 to 8fba7a2 Compare June 17, 2020 16:46
@rullzer
Copy link
Member

rullzer commented Jun 18, 2020

CI says no

@ChristophWurst
Copy link
Member Author

Christoph says yes

@ChristophWurst ChristophWurst force-pushed the enhancement/crash-reporters-registration-boostrap branch from 8fba7a2 to bde8ee7 Compare June 18, 2020 09:32
@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 18, 2020
@rullzer
Copy link
Member

rullzer commented Jun 18, 2020

Christoph says yes

🐨 🚀

@rullzer
Copy link
Member

rullzer commented Jun 19, 2020

CI still says no...
https://drone.nextcloud.com/nextcloud/server/29862/10/4

🐨 💣

@ChristophWurst ChristophWurst force-pushed the enhancement/crash-reporters-registration-boostrap branch from bde8ee7 to 2b7b714 Compare June 19, 2020 08:38
@ChristophWurst
Copy link
Member Author

Christoph says yes, again.

Turns out I did not check the registry tests and I even found an uncovered edge case while writing the tests 🐨 🚀

@rullzer
Copy link
Member

rullzer commented Jun 19, 2020

🐨 🎈

@rullzer rullzer merged commit 52dd1e3 into master Jun 19, 2020
@rullzer rullzer deleted the enhancement/crash-reporters-registration-boostrap branch June 19, 2020 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement pending documentation This pull request needs an associated documentation update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants