openfisca / openfisca-web-api

[DEPRECATED] Web API for OpenFisca
https://www.openfisca.fr/
GNU Affero General Public License v3.0
13 stars 11 forks source link

Update production deployment config #85

Closed cbenz closed 7 years ago

cbenz commented 7 years ago

Connected to #82.

CC @pndsagora

cbenz commented 7 years ago

I wait for a reviewer to respond, to be sure it's understandable, then I merge.

MattiSG commented 7 years ago

I am not competent to review this

@benjello If you don't feel competent, please don't “approve”, just “comment” 🙂 This makes it easier to see at a glimpse who reviewed what.

@cbenz do you think it would be possible to request reviews from slightly less people on all PRs? This notifies all the time and makes it harder to filter the noise from the parts where a review is actually requested 🙂

MattiSG commented 7 years ago

@pndsagora a review of these instructions would be very welcome to check that they would have solved the issues you mentioned in #82! :)

fpagnoux commented 7 years ago

@cbenz this seems almost ready to merge.

cbenz commented 7 years ago

This is GTM. But, as seen with @fpagnoux, there is an error on Travis, related to periods.ETERNITY. This happens because there is no constraint on OpenFisca-France in OpenFisca-Web-API.

I currently don't know with pip how to select the latest version of OpenFisca-France compatible with the latest version of OpenFisca-Core satisfying the constraints expressed in OpenFisca-Web-API.

fpagnoux commented 7 years ago

image

😱

cbenz commented 7 years ago

@fpagnoux How to do, if we forbid rebase + force push? The only left solution is to merge master back in feature branch, no?

Of course (and that's the point) we mustn't forget to rebase before merging, that's dangerous.

@MattiSG how to proceed?

Edit: https://github.com/openfisca/openfisca-core/pull/469#issuecomment-285336542

fpagnoux commented 7 years ago

I'll add it somewhere in the guidelines, but to answer the question here :

The workflow is :

fpagnoux commented 7 years ago

If this PR was only documentation change, a vesion bump was probably unnecessary.