I think it would be great to properly document this knowledge in writing! The checklists will probably be unique for django-cms vs the various plugins hosted under https://github.com/divio/* since they each have their own unique gotchas and architectural concerns.
I'm just hoping to formalize the process so that everyone has a handy reference when reviewing pull requests which might help them catch issues before they are merged into a release. I'm not a core developer, so I defer to others on the specifics of how this is accomplished. Thanks! On Sunday, October 23, 2016 at 11:13:51 PM UTC-7, Angelo Dini wrote: > > Hello @John > > We do have some sort of checklist though it's mostly shared verbally > between the core developers. > I think it would make sense to eventually further expand > http://docs.django-cms.org/en/release-3.4.x/contributing/code.html > > any other opinion? > > Cheers > Angelo > > On Saturday, 22 October 2016 06:08:08 UTC+2, John-Scott wrote: >> >> Is there any sort of code review checklist used internally by the core >> developers? Something that could be referenced by reviewers before >> approving changes, e.g. 'If this change modifies a public API call >> signature, be sure to clearly document the impact/follow the deprecation >> policy/etc'. >> >> It's easy to quickly scan a pull request and give a :thumbsup: but it may >> not be immediately apparent to the reviewer that there are gotchas to >> consider. I accept that we'll always run into unanticipated issues (such is >> life!), but I'd like to see these 'lessons learned the hard way' turned >> into a formal checklist so reviewers will be prompted to look for these >> types of issues going forward. >> >> Concretely, I've been bitten several times over the years by backwards >> incompatible changes that were not clearly identified in the release >> notes/history/changelog of django-cms and related plugins. >> >> A recent example is described in this djangocms-link issue >> <https://github.com/divio/djangocms-link/issues/101> and pull request >> <https://github.com/divio/djangocms-link/pull/102>. In this case, the >> default template changed paths and the context variables also changed. >> However, the changelog only said "* Cleaned up file structure". It wasn't >> clear at all from this that this impacted those of us who had customized >> the default template. Users are left to either experience the bug in >> production after what was assumed to be an innocent upgrade, or they have >> to be extremely thorough and review every git commit before upgrading. >> >> A checklist or some such document could be helpful in bringing structure >> to how future changes are evaluated. >> >> Thoughts? >> >> -- Message URL: https://groups.google.com/d/msg/django-cms-developers/topic-id/message-id Unsubscribe: send a message to django-cms-developers+unsubscr...@googlegroups.com --- You received this message because you are subscribed to the Google Groups "django CMS developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-cms-developers+unsubscr...@googlegroups.com. To view this discussion on the web, visit https://groups.google.com/d/msgid/django-cms-developers/041c7881-7c8c-4604-9a3f-5cfe77c13731%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
