-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue 82 erro portlet calendario idgserpro #87
Conversation
portlet_calendar.js foi customizado. Ler o relato para entender as motivações. Customizado a partir de https://raw.githubusercontent.com/plone/plone.app.event/1.1.8/plone/app/event/portlets/portlet_calendar.js
O último upgradeStep da branch 1.x é o 4003: https://github.com/plonegovbr/brasil.gov.agenda/tree/dd4fcfa103d625a1ee7f44cba9b6457e2a6b6a17/src/brasil/gov/agenda/upgrades E o primeiro da branch master (2.0) é o 4004: https://github.com/plonegovbr/brasil.gov.agenda/tree/58370bbb9c5a9182c8979aba7669bb09c10d9b82/src/brasil/gov/agenda/upgrades Dessa forma, nomearemos o upgradeStep na branch 1.x de 4003r01 para evitar conflitos com nomes. Essa lógica é parecida com a usada nos revisions dos UnifiedInstallers: https://launchpad.net/plone/4.3/4.3.15/+download/Plone-4.3.15-UnifiedInstaller-r2.tgz E também é reconhecida pelos padrões definidos em distutils.version.LooseVersion e packaging.version.parse.
Fecha #82. Patch foi feito da forma tradicional e não com uso de collective.monkeypatcher, ver https://pypi.org/project/collective.monkeypatcher/#patching-module-level-functions
Após discussões sobre padrões de números de upgradeSteps em plonegovbr/brasil.gov.portal#471 (comment) Decidimos mudar para 4004. Modificações futuras no brasil.gov.agenda com relação ao upgradeStep hoje definido na branch master (futuro 2.x) serão feitas em #86
@idgserpro antes de revisar o pull request gostaria entender porque isso não foi implementado upstream ao invés de parchar; isso pode ter implicações no futuro e também na versão 2.0. |
Ver a justificativa em plonegovbr/brasil.gov.portal#412 (comment) - lá fala dos riscos no futuro e com relação ao 2.0. |
Ler também #82. Além disso, plonegovbr/brasil.gov.portal#474 também precisa ser mesclado para atender #82 por completo. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, mas antes de mesclar gostaria da opinião de mais algum dos envolvidos.
No description provided.