search

collective / collective.nitf

A Dexterity-based content type inspired on the News Industry Text Format specification
8 stars 3 forks source link

Issue 200 - Remove dependency on Cycle 2 #212

Closed rodfersou closed 5 years ago

rodfersou commented 7 years ago

Closes #200

idgserpro commented 7 years ago

Can we have a 2.1b3 release and then a 2.2b1 release with this change (@rodfersou woul only need a rebase here) ? @hvelarde this is a relatively breaking change, we're just finishing upgrading collective.nitf in IDGB and won't have time to properly merge/review this.

hvelarde commented 7 years ago

@idgserpro this is far from complete; can you rise some points (besides the obvious changes on the template) that you consider we need to be careful?

my idea is to get rid of both Galleria and Cycle2 in all of our packages in the short term as one is a PITA and the other is no longer maintained; that includes this package, collective.cover and sc.photogallery, AFAIK.

idgserpro commented 7 years ago

The same warning that you gave in collective.cover (and here also for collective.z3cform.widgets) about removal of dependencies it would be nice, since it registers a browserlayer. If you already have collective.nitf installed and upgrades to this without having collective.js.cycle2, it'll break.

We're saying that we won't have time to merge not because this is a huge change, but because we customized a bunch of your templates and have already reviewed them but for collective.nitf 2.1b2. The PR for brasil.gov.portal and brasil.gov.tiles will be pushed today to github. A 2.1b3 release would be nice with the small fixes we did in the last PRs.

Changing js frameworks is like walking on eggs, so we don't know from a "memory" perspective what would be important to avoid issues: we suggest testing, by hand, an instance with collective.nitf 2.1b2, than upgrading to this, without cleaning caching, nothing, to see if you forgot something. I think the other obvious issues you already said in the code review.

hvelarde commented 7 years ago

@idgserpro I understand your concerns; I think we can wait before merging this and work in sc.photogallery in the mean time.

idgserpro commented 7 years ago

@hvelarde You can do a release for collective.nitf from what's on master, rebase here and continue, but bumping the version to 2.2 because of the nature of the PR. Then you don't have to rush on this PR.

rodfersou commented 5 years ago

@hvelarde is it possible to merge this before update static resources here?