CartoDB / carto.js

CartoDB javascript library
BSD 3-Clause "New" or "Revised" License
424 stars 264 forks source link

Investigate a good way to version CARTO.js while developing next release #1765

Closed alonsogarciapablo closed 6 years ago

alonsogarciapablo commented 7 years ago

https://github.com/cartodb/cartodb and https://github.com/cartodb/deep-insights.js are pointing to v4 branch. The main issue with this is that we might merge some "breaking" changes into v4 and projects that depend on CartoDB.js (i.e: BUILDER and deep-insight.js) might break when their dependencies are updated.

We need a mechanism to ensure BUILDER and deep-insights.js point to an specific version / tag / commit.

One option that was discussed was using git tags for this.

We should use this same versioning system in deep-insights.js.

alonsogarciapablo commented 7 years ago

cc: @IagoLast @ivanmalagon @Jesus89.

As part of this discussion, we can decide if yarn.lock should be committed.

IagoLast commented 7 years ago

As part of this discussion, we can decide if yarn.lock should be committed.

In a world where the package.json deps pointed to branches the developers were never sure about the versions being installed leading to malfunctions. yarn.lock ensures consistent builds freezing the dependency tree so the next person downloading the repo will install exactly the same versions.

But remember:

https://yarnpkg.com/blog/2016/11/24/lockfiles-for-all/

We need a mechanism to ensure BUILDER and deep-insights.js point to an specific version / tag / commit.

IMO we should publish a TAG after each merge on v4 while cartodb and deep-insights should point to a tag.

Also deep insights should have a new tag only changing the cartdob.js version pointed in the package.json whenever cartodb.js is has a new tag.

In the future we should move to using npm packages and semver allowing cartodb and deep-insights point to a range so we dont need to update them on every merge.

ivanmalagon commented 6 years ago

Summoning all @CartoDB/frontend to get their thoughts about this problem that affects us all.

Jesus89 commented 6 years ago

I think we have a great opportunity to move to npm for CARTO.js and deep-insights.js.

The problem of putting tags everywhere in each change is already solved. In our projects, with a little change in CARTO.js you need to add a tag in CARTO.js, deep-insights.js and also update this deps in the Builder. I think we can jump to npm and avoid suffering.

The npm packages should be non-public, I mean not to tell in public until 4.0.0 release. But we can enjoy the benefits of npm and semver!

Just update the README.md files and go ahead! :)

nobuti commented 6 years ago

Tags seem the logical path. This won't stop us to introduce a bug or breaking change, but at least we can easily "revert" it linking to a previous tag branch. Not sure about the yarn thing though. I'd like to use the same tools in every repo. For instance, linking some local repos is a PITA when they use yarn and Builder ends using npm.

IagoLast commented 6 years ago

@Jesus89 before uploading to npm we must:

@nobuti yarn and npm should be equivalent (yarn is faster compared to old npm versions) but you must use it consistently. yarn link wont work mixed with npm link

IMO the real PITA is mixing yarn.lock with npm lockfiles !

Jesus89 commented 6 years ago

Let's go with tags then! :muscle:

xavijam commented 6 years ago

AJAM!

xavijam commented 6 years ago

I makes sense guys, I feel @IagoLast and @rubenmoya already took a good decision about it and I'm totally with you ❤️ (except with @Jesus89 because he has just told me 👅 ).

Jesus89 commented 6 years ago

https://docs.google.com/document/d/1z27qj-IbtRl4rQF3wLVek7htrCmAVeN8KDj31XWnHDA/edit#heading=h.qt53u7yrnzk3