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

Standardise deprecations in CMS4 for other core modules #10531

Closed
3 tasks done
emteknetnz opened this issue Oct 7, 2022 · 2 comments
Closed
3 tasks done

Standardise deprecations in CMS4 for other core modules #10531

emteknetnz opened this issue Oct 7, 2022 · 2 comments
Assignees

Comments

@emteknetnz
Copy link
Member

emteknetnz commented Oct 7, 2022

Follow on card from #10500

Process deprecations in CMS 4 for all core modules except framework

AC

  • Code with a @deprecated annotation throw a Deprecation::notice()
  • Code throwing a Deprecation::notice() is annotated with a @deprecated
  • Normalise the @deprecated annotation ... within reason.

Implementation lessons from #10500

  • Use http://github.com/emteknetnz/silverstripe-deprecator
  • This will highlight deprecation messages that need to be manually changed - docblock has precedence over Deprecation::notice()
  • Undo code-writing commit, manually update messages, commit, then do 2nd commit containing code-writing

Related

PRs

@emteknetnz emteknetnz self-assigned this Oct 7, 2022
@emteknetnz emteknetnz changed the title Process deprecations in CMS4 for other core modules Standardise deprecations in CMS4 for other core modules Oct 12, 2022
@emteknetnz emteknetnz removed their assignment Oct 20, 2022
@GuySartorelli GuySartorelli self-assigned this Oct 24, 2022
@GuySartorelli
Copy link
Member

GuySartorelli commented Oct 24, 2022

@emteknetnz
Now that the version numbers aren't actually being checked, it would be tidiest if we changed the 4.0.1 deprecation notices back to 4.0.0 - that said, there's very little benefit in that beyond pedantic correctness so I won't actually ask you to make this change, just pointing out that it's a possibility now in case you want to do it.

@GuySartorelli GuySartorelli removed their assignment Oct 25, 2022
@GuySartorelli
Copy link
Member

PRs are merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants