Skip to content
This repository has been archived by the owner on Apr 28, 2022. It is now read-only.

Replacing duojs with npm and then updating the core library and way integrations are included. #514

Conversation

rcline
Copy link

@rcline rcline commented Jun 28, 2016

This is following all the recent work being done in analytics.js-core, analytics.js-integration, and an example integration (GTM).

  • Removed duojs
  • Using npm to bring in analytics.js-core and the integrations
  • Using Browserify to packed the export
  • Updated eslint
  • Made similar package.json changes that where made to other repos

@rcline
Copy link
Author

rcline commented Jun 28, 2016

@wcjohnson11, @ndhoule:

This is to update the Analytics.js build repo with all the changes to the core and integrations repos. Those changes actually broke the older tags of this repo so this fix is fairly high priority since building off master is now broken. I can go into more detail if needed.

Thanks for the consideration.
Ryan

@rcline rcline mentioned this pull request Jun 28, 2016
@rcline
Copy link
Author

rcline commented Jun 28, 2016

Looks like tests are failing:

$ make test-sauce
make: *** No rule to make target `test-sauce'.  Stop.
The command "make test-sauce" exited with 2.

I may need some help here.

@rcline rcline force-pushed the replacing-duojs-with-npm-build-and-updated-libraries branch 3 times, most recently from 0b040d6 to c7cde11 Compare June 28, 2016 15:04
…ntegrations are included. This is following all the recent work being done in analytics.js-core, analytics.js-integration, and an example integration (GTM).
@rcline rcline force-pushed the replacing-duojs-with-npm-build-and-updated-libraries branch from c7cde11 to 53a33ff Compare June 29, 2016 16:22
@rcline
Copy link
Author

rcline commented Jun 29, 2016

Got the build passing but just made a test-sauce command that calls test. Someone should probably review.

@arrygoo
Copy link

arrygoo commented Jul 7, 2016

+1
@rcline Sweet fix. I have tested this and it fixes our issue: https://segment.forumbee.com/t/638rmz/error-loading-the-analytics-min-js-file-this-morning-5272016

@sperand-io
Copy link
Contributor

@rcline this is awesome — thank you! I know @ndhoule had some big plans for the next major release of public analytics.js once we finish our internal migration. essentially removing the implicit dependence on all the integrations and providing better options for building custom builds through npm/browserify with only the ones you want. i'll let him follow up here!

@ndhoule
Copy link
Contributor

ndhoule commented Jul 7, 2016

@rcline Thanks a lot for this work! We're currently in the process of rolling Browserified analytics.js out across the Segment product and updating open-source Analytics.js to match will be my first order of work after that's completed. Definitely happy to jump in here and help get tests passing.

We're aiming to have our rollout finished this week and I'll follow up shortly thereafter.

@arrygoo
Copy link

arrygoo commented Jul 12, 2016

@ndhoule Do you think this might get merged in the near future? I'm only asking because we are wondering if we should merge this locally to fix the issues on our wrap of this package or wait few days.

@rcline
Copy link
Author

rcline commented Jul 12, 2016

Thanks @sperand-io, @ndhoule,

I am very glad this is getting attention and all the great work being done to update this collection of tooling.

My company is using a version of this branch and it is working very well. Feel free to use this as a starting point for the work you have planned or start off master. Either way I will be looking forward to the update and to keep my fork in sync with the changes.

Much thanks!

@rcline
Copy link
Author

rcline commented Jul 14, 2016

@arrygoo I would recommend either forking yourself or using my fork/branch for now. This will get you unblocked in the short term. Also, since this is a "Build repo" with the output being a single file it should be fairly painless to upgrade when segment has made the official v3 release.

@arrygoo
Copy link

arrygoo commented Jul 14, 2016

Thanks @rcline, I'm using your fork/branch on https://github.com/okgrow/analytics until v3 release.

@tanzim
Copy link

tanzim commented Nov 8, 2016

We're aiming to have our rollout finished this week and I'll follow up shortly thereafter.
Any update since July @ndhoule?

@yegortokmakov
Copy link

hola. any news on this issue?

@ayb
Copy link

ayb commented Jan 9, 2017

Just got the same build error today and found this PR

Any plans to merge it into master?

@rcline
Copy link
Author

rcline commented Jan 10, 2017

@ayb I have not heard anything from the segment team lately, I am sure they have their hands full. I would recommend just using this fork/branch (or fork off this one yourself, adding/removing your specific integrations) for now until an update is made. Since it is just a building repo with a single artifact file it should be easy enough to swap out once an official update is released. Let me know if you have questions!

@ayb
Copy link

ayb commented Jan 10, 2017

@rcline Got it working this morning. Thanks :)

@danieleleoni
Copy link

Hi guys, any update? It seems that this repository is not very often updated

cfnelson added a commit to okgrow/analytics.js that referenced this pull request Sep 21, 2017
@elgreg
Copy link

elgreg commented Nov 15, 2017

Using this fork as well. Thanks!

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job with updating the minifier. This should fix the minified bundles not being property minified.

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.