-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add deprecation policy #7199
Add deprecation policy #7199
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7199 +/- ##
==========================================
- Coverage 93.90% 93.89% -0.01%
==========================================
Files 181 181
Lines 13194 13194
==========================================
- Hits 12390 12389 -1
- Misses 804 805 +1
Continue to review full report at Codecov.
|
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!
@davimacedo Just to highlight 2 things:
Seems OK to me, what do you think? |
We can still launch a major version collecting a breaking change A that was introduced more than 6 months ago and not collecting a breaking change B that was introduced less than 6 months ago, right? |
Yes! |
I think it's a good idea but I have a few queries...
If notification for 1 breaking change happened 5 months ago and the other 6 months ago you could have two major releases 1 month apart? So not min 6 months... Also we don't currently have a publicly stated 'versioning system' (e.g. semver) that mandates breaking changes being a major release. It would probably make sense to clear that up alongside this. In terms of notification, I wonder how likely devs are to see a warning log? and whether we should use other means to notify - e.g. Parse Dashboard, forum, slack channel. I suppose now we ask people to add their changes to the the changelog that could also have a record of upcoming breaking changes |
Hmm, it only should mean that a breaking change should be announced for at least 6 months before making it. Making the breaking change requires a major version increment. There could be other reasons for major version increments such as major feature additions, but I see this only as theoretical. If such an increment happens due to a major feature additions it would not contain any breaking changes that were not announced for at least 6 months.
You are correct. That was one of my intentions, but beyond a versioning system, I want to slowly push towards a release cycle system. I have mentioned this several times and I think it helps developers and us to better plan. I didn't go all the way here because the suggestion did not resonate much in the past, but I think as we are getting there, the advantages becomes more apparent. Ideally, we go with semver:
A simple release cycle, for example:
I expect that in the process we can automate some of the release tasks, so that it becomes less work to actually do a release and we can drive down the times between minor and patch releases. But again, this PR is only about making breaking changes more foreseeable, which is a pain point often voiced in community feedback. We can open another PR and dive deeper into the topic of release cycle. In addition to warnings in the logs we would mention deprecations in the CHANGELOG, so that a developer can browse the log for current deprecations. |
I think we should come to a conclusion here. This suggestion of a phased deprecation policy already proofed to be a useful reference in instances where contributors were discussing features in the context of their breaking change effects. I suggest that we pass this and discuss the introduction of semver versioning as a next step on this front. After that we can discuss the introduction of fixed release cycles for which there is much potential for automation, from automatic changelog generation to versioning. |
Comment moved to #7271 |
To separate the discussion, I created a new meta issue #7271 on release cycle introduction. We have 3 issues here:
The release cycle issue picks up the versioning system as well. @sadortun Good input; you may want to move your comment to the other issue for discussion. |
@mtrezza thanks for your responses to my earlier queries, glad to see you are keen to improve the other related factors we could end up with a nice smooth system when all done. I broadly support this, my only remaining concern is any extra burden being placed on maintainers and or contributors. I've listed a few potential issues which come to mind and would welcome opinions on this. I'm not best placed to pass judgment on this so if @mtrezza, @dplewis, @davimacedo feel any additional burden is manageable or outweighed by the benefits I'll happy support this.
|
Pros:
Cons:
|
@TomWFox Let me know if #7199 (comment) addresses your points. I see the pros outweigh the cons and if we discover an aspect that requires too much effort, we can address it as we go along. Phased deprecation is an essential element in moving to fixed release cycles. I suggest, let's aim for the best and if there is an aspect that requires too much effort, then we address it as we go along. This policy is really just a nice way of saying, "if you break, then your feature will have to wait until the next major release or you can make it optional and add the feature sooner with the next minor release. However, if you intend to break an essential feature, then we likely don't accept it without phasing it in, because the risk is too high to release fundamental changes without getting an appropriate amount of feedback first". There wording still needs some adaption, as we now have a centralized Deprecator to make phased deprecation easier for contributors. |
@mtrezza Yes, thanks I'd say my my points have been mostly addressed.
I would say you and some others could be considered 'Super Contributors' 😉. Not all contributors can be expected to operate with your level of forward & long term thinking - and even 'Super Contributors' falter at some point! However, I do see this as being a beneficial direction to move in long term and ultimately we won't know the impact for considerable time (and may not be measurable anyway). As you said we can address issues as and when, so let's see how it goes!
Nice way to articulate it. |
@TomWFox Yes, I expect an intuitive branch concept to aid in the long-term thinking. It's difficult currently, because we only have a |
I would say, if @dplewis is also fine with this PR, then we'll go ahead and merge. |
Sorry guys took a small vacation. I approve of this approach. Most developers know what major, minor, patch means but making that clear for new developers is a plus. Some breaking changes have slipped though the cracks to a minor release but is very few. This will definitely mitigate that. I just update to 4.5.0 last week and I was kind of afraid to. Most major releases happened because of deprecating database / node major releases (Mongo 3.2 and 3.4 and node 8 in 4.0.0), the addition of new indexes (4.0.0), or heavy rewrites (removing bare bone in 3.0.0) just to mention a few to add to the PR. I have a PR in mind that will improve a new live query feature performance for high traffic applications but will be a breaking change. Let’s say I add it after the 5.0.0 release. What would that look like? |
@dplewis welcome back and thanks for the detailed comment.
Ideally:
In rare cases where it is not feasible to maintain an optional feature:
As you can see, the 2nd process is much shorter, but even though the breaking change happens correctly with a major release, it is a sudden breaking change. We would only allow this if the nature of the change would implicate a high level of complexity or effort to phase it in. This shorter process means we will not get as much feedback to mature the feature and create a high risk for bugs and unconsidered side effects. Especially when it comes to fundamental changes that are risky or likely to affect a high number of deployments, we would not allow the shorter process but always use phasing-in. The schema cache change would be an example where we would have phased-in according to this new policy rather than making a sudden breaking change. Side note: this policy does not apply to what we currently call "experimental features", that are features of |
@dplewis It says you have requested changes in your review, but I cannot find which changes have been requested. Is there anything specific or do you have any feedback to the general process outlined in the previous comment? |
@dplewis @TomWFox @davimacedo If there are no objections, I will merge this PR later today. We already have the Deprecator merged into Parse Server and it is already being used to deprecate features. This PR provides the official policy how to deprecate. It is also an essential step towards fixed release cycles and release automation. |
Can you hold off for now, I haven't had a chance to review the latest changes. |
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 👍 apart from my single comment.
f502c45
to
20172ce
Compare
* fix: upgrade mongodb from 3.6.3 to 3.6.5 Snyk has created this PR to upgrade mongodb from 3.6.3 to 3.6.5. See this package in npm: https://www.npmjs.com/package/mongodb See this project in Snyk: https://app.snyk.io/org/acinader/project/8c1a9edb-c8f5-4dc1-b221-4d6030a323eb?utm_source=github&utm_medium=upgrade-pr * bump mongo 3.6.6 * update package-lock * updated package-lock * fix: upgrade winston-daily-rotate-file from 4.5.0 to 4.5.1 (#7309) Snyk has created this PR to upgrade winston-daily-rotate-file from 4.5.0 to 4.5.1. See this package in npm: https://www.npmjs.com/package/winston-daily-rotate-file See this project in Snyk: https://app.snyk.io/org/acinader/project/8c1a9edb-c8f5-4dc1-b221-4d6030a323eb?utm_source=github&utm_medium=upgrade-pr * Bump CI environment, remove Postgres 10 support (#7323) * bumped MongoDB to 4.4.5 * bump Node to 14.16.1 * removed obsolete COVERAGE_OPTION * improved postges support note * bump more node * Remove MongoDB 3.6 support (EOL) (#7315) * removed mongodb 3.6 support * add changelog entry * updated CI check * bumped MongoDB to 4.4.5 * bump Node to 14.16.1 * removed obsolete COVERAGE_OPTION * improved postges support note * bump more node * updated package lock * Revert "bumped MongoDB to 4.4.5" This reverts commit ce9c810. * skipping MongoDB 4.4.5 temporarily * fixed bug in CI check that did not consider ignored versions when checking for newer versions * removed Postgres 10 support * updated Postgres versions * renamed MongoDB CI tests * fixed Postgres compatibility table * fix Postgres badge * Add deprecation policy (#7199) * added phased deprecation policy * fixed typo * added changelog entry * some rewording * Fixed typo * fixed typo * Fixed typo * updated deprecation policy * remove empty line * fix: upgrade mongodb from 3.6.3 to 3.6.5 Snyk has created this PR to upgrade mongodb from 3.6.3 to 3.6.5. See this package in npm: https://www.npmjs.com/package/mongodb See this project in Snyk: https://app.snyk.io/org/acinader/project/8c1a9edb-c8f5-4dc1-b221-4d6030a323eb?utm_source=github&utm_medium=upgrade-pr * bump mongo 3.6.6 * Update package-lock.json Co-authored-by: Manuel Trezza <[email protected]>
* added phased deprecation policy * fixed typo * added changelog entry * some rewording * Fixed typo * fixed typo * Fixed typo * updated deprecation policy * remove empty line
* fix: upgrade mongodb from 3.6.3 to 3.6.5 Snyk has created this PR to upgrade mongodb from 3.6.3 to 3.6.5. See this package in npm: https://www.npmjs.com/package/mongodb See this project in Snyk: https://app.snyk.io/org/acinader/project/8c1a9edb-c8f5-4dc1-b221-4d6030a323eb?utm_source=github&utm_medium=upgrade-pr * bump mongo 3.6.6 * update package-lock * updated package-lock * fix: upgrade winston-daily-rotate-file from 4.5.0 to 4.5.1 (parse-community#7309) Snyk has created this PR to upgrade winston-daily-rotate-file from 4.5.0 to 4.5.1. See this package in npm: https://www.npmjs.com/package/winston-daily-rotate-file See this project in Snyk: https://app.snyk.io/org/acinader/project/8c1a9edb-c8f5-4dc1-b221-4d6030a323eb?utm_source=github&utm_medium=upgrade-pr * Bump CI environment, remove Postgres 10 support (parse-community#7323) * bumped MongoDB to 4.4.5 * bump Node to 14.16.1 * removed obsolete COVERAGE_OPTION * improved postges support note * bump more node * Remove MongoDB 3.6 support (EOL) (parse-community#7315) * removed mongodb 3.6 support * add changelog entry * updated CI check * bumped MongoDB to 4.4.5 * bump Node to 14.16.1 * removed obsolete COVERAGE_OPTION * improved postges support note * bump more node * updated package lock * Revert "bumped MongoDB to 4.4.5" This reverts commit ce9c810. * skipping MongoDB 4.4.5 temporarily * fixed bug in CI check that did not consider ignored versions when checking for newer versions * removed Postgres 10 support * updated Postgres versions * renamed MongoDB CI tests * fixed Postgres compatibility table * fix Postgres badge * Add deprecation policy (parse-community#7199) * added phased deprecation policy * fixed typo * added changelog entry * some rewording * Fixed typo * fixed typo * Fixed typo * updated deprecation policy * remove empty line * fix: upgrade mongodb from 3.6.3 to 3.6.5 Snyk has created this PR to upgrade mongodb from 3.6.3 to 3.6.5. See this package in npm: https://www.npmjs.com/package/mongodb See this project in Snyk: https://app.snyk.io/org/acinader/project/8c1a9edb-c8f5-4dc1-b221-4d6030a323eb?utm_source=github&utm_medium=upgrade-pr * bump mongo 3.6.6 * Update package-lock.json Co-authored-by: Manuel Trezza <[email protected]>
🎉 This change has been released in version 5.0.0-beta.1 |
🎉 This change has been released in version 5.0.0 |
New Pull Request Checklist
Issue Description
As often pointed out in community feedback, Parse Server at times introduced breaking changes that were unnecessarily sudden. This can lead to a number of issues, also evident in previously opened issues:
Related issue: #7271
Approach
Adds a Phased Deprecation Policy that governs how to change / remove existing features and when to release breaking changes.
TODOs before merging