Skip to content
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

Merged
merged 10 commits into from
Apr 8, 2021
Merged

Conversation

mtrezza
Copy link
Member

@mtrezza mtrezza commented Feb 16, 2021

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:

  • "upgrade fatigue" where developers run old versions of Parse Server because they cannot always attend to every update that contains a breaking change
  • less secure Parse Server deployments that run on old versions which is contrary to the security evangelism Parse Server intends to facilitate for developers
  • less feedback on feature, slower identification of bugs and an overall slow-down of Parse Server development because new versions with breaking changes also include new features we want to get feedback on

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

  • Add entry to changelog

@codecov
Copy link

codecov bot commented Feb 16, 2021

Codecov Report

Merging #7199 (b288bac) into master (8ba3f02) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            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     
Impacted Files Coverage Δ
src/batch.js 91.37% <0.00%> (-1.73%) ⬇️
src/ParseServerRESTController.js 97.01% <0.00%> (-1.50%) ⬇️
src/RestWrite.js 93.92% <0.00%> (-0.16%) ⬇️
src/Adapters/Files/GridFSBucketAdapter.js 80.32% <0.00%> (+0.81%) ⬆️
src/Adapters/Auth/httpsRequest.js 100.00% <0.00%> (+4.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ba3f02...b288bac. Read the comment docs.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Member

@davimacedo davimacedo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mtrezza
Copy link
Member Author

mtrezza commented Feb 16, 2021

@davimacedo Just to highlight 2 things:

  • Breaking changes are collected into the next major release of Parse Server.
    --> That means that we do not introduce any breaking changes until the next major release of Parse Server (unless security relevant).

  • Developers should be notified of breaking changes for at least 6 months before they become mandatory.
    --> That means that major Parse Server release have to be at least 6 months apart.

Seems OK to me, what do you think?

@davimacedo
Copy link
Member

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?

@mtrezza
Copy link
Member Author

mtrezza commented Feb 17, 2021

Yes!

@TomWFox
Copy link
Contributor

TomWFox commented Feb 19, 2021

I think it's a good idea but I have a few queries...

Developers should be notified of breaking changes for at least 6 months before they become mandatory.
--> That means that major Parse Server release have to be at least 6 months apart.

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

CHANGELOG.md Outdated Show resolved Hide resolved
@mtrezza
Copy link
Member Author

mtrezza commented Feb 20, 2021

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...

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.

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.

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:

  • Breaking change, significant feature additions, UI redesign be a major increment.
  • Feature additions be a minor increment.
  • Bug fixes be a patch increment.

A simple release cycle, for example:

  • A major release every 6 month.
  • A minor release every 3 months
  • A patch release every month or earlier if the fix is at least a bug of severity S2.

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.

@mtrezza
Copy link
Member Author

mtrezza commented Mar 8, 2021

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.

@dplewis @TomWFox your thoughts?

@sadortun
Copy link
Contributor

sadortun commented Mar 16, 2021

Comment moved to #7271

@mtrezza
Copy link
Member Author

mtrezza commented Mar 16, 2021

To separate the discussion, I created a new meta issue #7271 on release cycle introduction.

We have 3 issues here:

  • Phased deprecation policy
  • Versioning system
  • Release cycle

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.

@TomWFox
Copy link
Contributor

TomWFox commented Mar 16, 2021

@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.

  1. Time required to make an otherwise breaking change optional and defaulting to off (contributor burden)
  2. Time required to make change default to on / remove optionality (contributor or maintainer burden) - perhaps more likely to be maintainer due to 6 month time gap - do we need to maintain a list of such changes required in 6 months?
  3. Any additional time spend reviewing / assisting with PRs for both 1 & 2 (maintainer burden)

@mtrezza
Copy link
Member Author

mtrezza commented Mar 17, 2021

Pros:

  • Solves pain point that has been mentioned repeatedly in community feedback
  • Decouples breaking changes from new features, improvements & bug fixes
  • Developers are more likely to upgrade due to gradual breaking changes (only in major versions)
  • Forces us to think more modular in PRs to turn on/off segments of code, which is the essence of Parse Server anyway
  • Refreshes code more frequently, as code blocks are more likely to be written from ground up instead of old code being hacked / modified and dragged along
  • We have experience with optional features (phasing-in instead of out), so we should be able to estimate the risk of additional effort quite accurately. See experimental features like directAccess, singleSchemaCache, PagesRouter. For example, for the PagesRouter I already added a TODO list to the PR for what needs to be changed to move it out of experimental status, so it will easy to pick this up again in a few months. There is not much difference in work volume, had I done it immediately.
  • Provides feedback period for optional feature and lowers risk of braking something

Cons:

  • Splits a PR into 2 PRs: a PR for the deprecation warning and optional change, another to remove the deprecated code; which means more admin / review effort
  • If the second PR are not done by the original contributor(s), it could take more effort for someone unfamiliar with the changes to do the PR.

@mtrezza
Copy link
Member Author

mtrezza commented Mar 30, 2021

@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.

@TomWFox
Copy link
Contributor

TomWFox commented Mar 30, 2021

@mtrezza Yes, thanks I'd say my my points have been mostly addressed.

We have experience with optional features (phasing-in instead of out), so we should be able to estimate the risk of additional effort quite accurately. See experimental features like directAccess, singleSchemaCache, PagesRouter. For example, for the PagesRouter I already added a TODO list to the PR for what needs to be changed to move it out of experimental status, so it will easy to pick this up again in a few months. There is not much difference in work volume, had I done it immediately.

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!

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".

Nice way to articulate it.

@mtrezza
Copy link
Member Author

mtrezza commented Mar 30, 2021

Not all contributors can be expected to operate [at a level] of forward & long term thinking

@TomWFox Yes, I expect an intuitive branch concept to aid in the long-term thinking. It's difficult currently, because we only have a master and no other branches that facilitate forward looking. That's a todo on the roadmap.

@mtrezza
Copy link
Member Author

mtrezza commented Mar 31, 2021

I would say, if @dplewis is also fine with this PR, then we'll go ahead and merge.

@dplewis
Copy link
Member

dplewis commented Apr 1, 2021

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?

@mtrezza
Copy link
Member Author

mtrezza commented Apr 1, 2021

@dplewis welcome back and thanks for the detailed comment.

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?

Ideally:

  1. Add it as optional feature anytime during 5.#.# with opt-in via a new Parse Server option. What seems to be a breaking change at first, may turn out to become a permanent optional feature instead if there are valid use cases for the existing feature, see directAccess as example. Add a deprecation warning.
  2. The optional feature will be advertised in our Feature Spotlight articles in the Community Forum to get many developers try it out and provide feedback. We are currently looking into creating a mailing list to reach a wider audience for new feature adoption.
  3. Developers are notified about the deprecation for at least 1 full major version by logging the deprecation warning, that's a few months of 5.#.# and a full year of 6.#.#. Note that during that time, the feature is not of alpha or beta maturity but starts with at least release candidate maturity and gradually reaches release maturity, so developers can already adopt this in their production environments and from that feedback we fix any remaining bugs.
  4. After adding the optional feature sometime during 5.#.# (2021), logging deprecation for the full major version of 6.#.# (2022), and given that the existing feature has no valid use case and should be removed, and given that developer feedback does not merit postponing the introduction, and given that the feature reached release maturity, the breaking change will happen with 7.0.0 (Jan 2023). The deprecation warning and any deprecated code will be removed.

In rare cases where it is not feasible to maintain an optional feature:

  1. Add the feature already as breaking change by replacing the existing feature. We still have to define a forward looking branch system to conveniently merge into a branch for 6.0.0 while we are on 5.#.#.
  2. The feature will be introduced in 6.0.0 (Jan 2022). It needs to have at least release candidate maturity, which is quite an effort in itself and should discourage from going the shorter process out of convenience. Allowing to break with release candidate maturity is more generous than in the longer process, but I think it would be illusional to expect a feature that has not been widely adopted and tested to reach a higher maturity.

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 alpha or beta maturity, which have to be defined in a separate policy. Historically, experimental features were used because we had no deprecation policy. That led to confusion about the maturity of experimental features, with developers sometimes using them in production. Experimental features were even suggested in comments to fix an issue, e.g. directAccess, which should be discouraged. Going forward, experimental features will be used like features with alpha or beta maturity that may not ever make it to a release, for example if they become outdated or do not work conceptually as imagined. Phased-in features via deprecation policy need to be at least of release candidate maturity.

@mtrezza
Copy link
Member Author

mtrezza commented Apr 2, 2021

@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?

@mtrezza
Copy link
Member Author

mtrezza commented Apr 7, 2021

@dplewis, @TomWFox do you see any remaining issues to address in this policy?

@mtrezza
Copy link
Member Author

mtrezza commented Apr 8, 2021

@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.

@TomWFox
Copy link
Contributor

TomWFox commented Apr 8, 2021

Can you hold off for now, I haven't had a chance to review the latest changes.

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@TomWFox TomWFox left a 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.

@mtrezza mtrezza merged commit a074fc9 into parse-community:master Apr 8, 2021
@mtrezza mtrezza deleted the add-deprecation-policy branch April 8, 2021 22:50
mtrezza added a commit that referenced this pull request Apr 8, 2021
* 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 <5673677+mtrezza@users.noreply.github.com>
Arul- pushed a commit to Arul-/parse-server that referenced this pull request Apr 12, 2021
* added phased deprecation policy

* fixed typo

* added changelog entry

* some rewording

* Fixed typo

* fixed typo

* Fixed typo

* updated deprecation policy

* remove empty line
Arul- pushed a commit to Arul-/parse-server that referenced this pull request Apr 12, 2021
* 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 <5673677+mtrezza@users.noreply.github.com>
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Nov 1, 2021
@mtrezza mtrezza mentioned this pull request Mar 12, 2022
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-beta Released as beta version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants