Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

Add HipChat (Connect) plugin #7

Merged
merged 10 commits into from Sep 2, 2016
Merged

Add HipChat (Connect) plugin #7

merged 10 commits into from Sep 2, 2016

Conversation

dcramer
Copy link
Member

@dcramer dcramer commented Aug 31, 2016

  • Works with both legacy UI as well as upcoming React changes (Expand plugin API (JS) sentry#4058)
  • Removes use of SENTRY_URL_PREFIX, which is invalid, but happens to work on sentry.io

@macqueen


This change is Reviewable

@dcramer
Copy link
Member Author

dcramer commented Aug 31, 2016

This still needs to manage asset loading. Right now it can generate the .js file, but theres nothing to do it automatically (e.g. as part of setup.py install), and there's nothing to actually load the .js in Sentry itself.

@dcramer
Copy link
Member Author

dcramer commented Aug 31, 2016

Of note, I'm going to test this against master, and if it works we can probably get the core of this in, and do a follow up PR which finishes the React transition (as it won't even be used in master right now).

@dcramer
Copy link
Member Author

dcramer commented Sep 1, 2016

Seems to still work fine without the React changes

@dcramer
Copy link
Member Author

dcramer commented Sep 1, 2016

Also I fixed like 10 </p> tags that @mitsuhiko forgot

@dcramer
Copy link
Member Author

dcramer commented Sep 2, 2016

FYI the only changes here are:

  • Remove SENTRY_URL_PREFIX (which adds the regexp cache, makes some things dynamic)
  • Add the JS prototype (unused right now)

@macqueen
Copy link
Contributor

macqueen commented Sep 2, 2016

Reviewed 31 of 38 files at r1, 1 of 2 files at r2, 9 of 9 files at r3.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


webpack.config.js, line 50 [r1] (raw file):

      path: path.join(__dirname, distPath),
      // libraryTarget: 'var',
      // library: ['Sentry', 'plugins', app],

any reason to not delete these?


src/sentry_plugins/hipchat_ac/static/hipchat_ac/components/settings.jsx, line 65 [r3] (raw file):

    let tenants = this.state.tenants;
    if (tenants.length === 0)

i'd just do if (!tenants.length)


Comments from Reviewable

@dcramer dcramer merged commit d7c732f into master Sep 2, 2016
@dcramer dcramer deleted the hipchat branch September 2, 2016 01:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants