Skip to content

Conversation

floriangosse
Copy link
Contributor

For #446

@benvinegar
Copy link
Contributor

Sweet, taking a look now.

@benvinegar
Copy link
Contributor

After I run grunt dist, I get the following:

Untracked files:
  (use "git add <file>..." to include in what will be committed)

    dist/angular.js
    dist/angular.min.js
    dist/angular.min.js.map
    dist/console.js
    dist/console.min.js
    dist/console.min.js.map
    dist/ember.js
    dist/ember.min.js
    dist/ember.min.js.map
    dist/require.js
    dist/require.min.js
    dist/require.min.js.map

... and an empty directory, dist/plugin.

Can we change it so that the plugins are built into dist/plugins/* (note plural plugins, matching src/plugins)?

The temp directory build/plugins should also be the plural form of plugins.

@benvinegar
Copy link
Contributor

@floriangosse – do you mind me asking, are you using Bower with RequireJS? Or just loading these scripts as-is?

@benvinegar
Copy link
Contributor

Also, I notice every file here has the same UMD wrapper, caused by this Browserify config.

After a discussion with @mattrobenolt, I think what we want is for each of these plugins to have its own unique UMD wrapper, whereby each plugin can be loaded via CommonJS, AMD (Require.js), or globally on the window via Raven.Angular, Raven.Console, Raven.Ember.

We'd also prefer to remove the auto-install behavior of the plugin build we created for the CDN; we did that to preserve backwards compatibility with CDN users, and it's not something we want to continue (we have a long-term goal of moving plugins into their own repositories so they can be versioned independently).

In that scenario, you'd have something like:

<script src="bower_components/dist/raven.js"></script>
<script src="bower_components/dist/plugins/angular.js"></script>
<script>
Raven.config('DSN').addPlugin(Raven.Angular).install();
</script>

At this point, we're talking about quite a lot of changes, so it's not necessary for you to put this together for us (we'll it). But if you want to charge forward anyways we'd be glad to assist.

@benvinegar
Copy link
Contributor

Closed in favor of #476, which was merged. I kept your original commit though @floriangosse – thanks again for your contribution.

@benvinegar benvinegar closed this Jan 18, 2016
@floriangosse floriangosse deleted the build-plugins branch October 18, 2016 07:46
kamilogorek added a commit that referenced this pull request Jun 12, 2018
* ref: use console.warn for alerts and store them in Set
* test: consoleAlert and consoleAlertOnce tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants