search

dschmidt / ember-cli-deploy-sentry

An ember-cli-deploy-plugin to upload javascript sourcemaps to Sentry
MIT License
42 stars 51 forks source link

Remove legacy API token and require bearer token #32

Closed RobinDaugherty closed 6 years ago

RobinDaugherty commented 7 years ago

Support for bearer tokens was added to the plugin, since that is required for modern Sentry support. But sentryApiToken is still required, which makes no sense. There are about 6 forks of the repo solely for the purpose of making this work.

I have removed legacy sentryApiToken support completely, requiring sentryBearerApiToken instead. I recommend releasing this with a major version bump, so we can move ahead. Those using the old version of Sentry will need to stick with their old version of the plugin, or update and get a new token from Sentry.

duizendnegen commented 7 years ago

Why don't we instead move sentryApiToken to optionalConfig and verify in the setup-step whether either of them is configured?

Then again, if the API token is strictly legacy, then going with the bearerApiToken forward & releasing 0.6 also makes sense.

@dschmidt your thoughts?

dschmidt commented 7 years ago

I'm with @duizendnegen here. Why should we break old sentry versions?!

mirague commented 7 years ago

Would be great to move forwards on this one, as the current release is simply broken for new adopters of the plugin.

RobinDaugherty commented 7 years ago

I suggest that the old version of Sentry doesn't need to be supported in the new version of the plugin. If someone is still running the old version of Sentry, they shouldn't upgrade. Hence the major version bump, assuming that we're following semantic versioning.

RuslanZavacky commented 6 years ago

Guys, any news on this one? This plugin is actually broken for new adopters. Would be lovely to fix it :)

duizendnegen commented 6 years ago

We are happy to merge in a PR with sentryApiToken as backward compatibility still being there, in the PR this is completely removed.

duizendnegen commented 6 years ago

Fixed by https://github.com/dschmidt/ember-cli-deploy-sentry/pull/43