search

lukeyouell / craft-sentry

Error tracking that helps developers monitor and fix crashes in real time. Iterate continuously. Boost efficiency. Improve user experience.
https://sentry.io
MIT License
17 stars 10 forks source link

Add ability to save an environment as a config param and send it with the captured exceptions #14

Closed Tyrannogina closed 5 years ago

Tyrannogina commented 5 years ago

User will be able to define a Sentry environment in their .env file so they can filter by environment the exceptions captured by Sentry. I didn't want to use the CRAFT_ENVIRONMENT since I might want to just define dev and production in Sentry independently if Craft environment is more segmented (local, staging, preproduction...). It defaults to it if the environment config is not set.

Some imports that did not seem to be used were deleted.

lukeyouell commented 5 years ago

Hi @Tyrannogina, this feature is available in the latest release!

Tyrannogina commented 5 years ago

You should really credit contributors instead of just taking and editing our contributions and pretend you wrote them from scratch. It's not that difficult to have us write our contributions towards a develop branch, and actually review our pull requests and ask for changes. And if you are going to take our code directly, the bare minimum would be that you ask us and explain why.

As my first open project contribution, it was an important PR for me, and I am really disappointed with this behaviour, and I really hope other repository owners do a better job in this regard.

lukeyouell commented 5 years ago

Hi @Tyrannogina - I'm sorry you feel this way, there definitely weren't any ill-intentions regarding this.

The latest release included a large proportion of the plugin being re-written from scratch, with the issues raised by this PR and two others in mind (I didn't take your code directly as you say) - so the conditions during the development of 1.5 were completely different to when you submitted the PR, which unfortunately meant reviewing/directly incorporating this PR would have taken up too much time due to it not being compatible with other changes made to the plugin, such as those made to achieve Craft CMS 3.1 compatibility.

I hope you can appreciate that as this is something I do in my own free time for a plugin that is completely free of charge, that I prioritise the amount of time it takes me to do so.

Tyrannogina commented 5 years ago

I know you didn't take my code verbatim and you did lots of changes, but a note stating your approach on the readme or on this PR prior to releasing would have been the nice thing to do.

It would also be nice to have a develop branch on the project with the changes you intend to implement in next releases so people can contribute there, instead of keeping the process obscured and then delete other people's contributions because they don't take into account things we didn't know were going to be implemented.

Aside on how the contributions are handled, I understand there's no release with the environment that works with Craft 3.0, right? I can't update my project to 3.1 yet due to other plug-ins we are using.

lukeyouell commented 5 years ago

That's a fair comment, and something I'll consider from now on.

Unfortunately there are limitations to this as when it comes to maintaining these free plugins as I have short periods of availability where I can dedicate time to work on them, so releases usually follow shortly after. This means that in most cases collaborative development like you suggested just isn't convenient.

Aside on how the contributions are handled, I understand there's no release with the environment that works with Craft 3.0, right?

That's correct.