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

Replace deprecated constants and methods in the extension package #717

Merged
merged 1 commit into from
Nov 7, 2018
Merged

Replace deprecated constants and methods in the extension package #717

merged 1 commit into from
Nov 7, 2018

Conversation

ggrossetie
Copy link
Member

  • @deprecated BlockProcessor context constants (replaced by Contexts)
  • should we remove Processor constants ?
  • Use diamond where possible Map<String, String> config = new HashMap<>()
  • Reformat code (tabs -> spaces)

Copy link
Member

@robertpanzer robertpanzer left a comment

Choose a reason for hiding this comment

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

Looks good to me and I would merge this.
Just want to make sure that the * imports are intended.

- @deprecated BlockProcessor context constants (replaced by Contexts)
- should we remove Processor constants ?
@ggrossetie
Copy link
Member Author

@robertpanzer should we remove Processor constants as they were already marked as @deprecated (in 2016 !)

I think we should write a rule to define when @deprecated features will be removed (or add an explicit notice on the annotation, "will be removed in 2.0.0").
For instance in Scala:

A deprecated element of the Scala language or a definition in the Scala standard library will be preserved at least for the current major version.
This means that an element deprecated in some 2.12.x release will be preserved in all 2.12.x releases, but may be removed in 2.13. (A deprecated element might be kept longer to ease migration. Developers should not rely on this.)

https://www.scala-lang.org/api/current/scala/deprecated.html

We could also add a @since so we know that this feature has been deprecated since version X (and since we are version Y it's pretty safe to remove it).

@robertpanzer
Copy link
Member

I would vote for removing it with 2.0.0.
There should be some users that are already on 1.6.0 for a longer time and we shouldn't break them like that.

Scala is a special case, it is just normal that it is incompatible with dot-releases, and therefore you see libraries available for Scala 2.11 and 2.12. So much for semantic versioning 😂

@robertpanzer robertpanzer merged commit e9b81c6 into asciidoctor:master Nov 7, 2018
@ggrossetie
Copy link
Member Author

ggrossetie commented Nov 7, 2018

Scala is a special case, it is just normal that it is incompatible with dot-releases, and therefore you see libraries available for Scala 2.11 and 2.12. So much for semantic versioning

My point was to define a rule regarding deprecated elements (we certainly don't want to use the same versioning strategy as Scala).
I think it's safe to use a rule where deprecated elements in 2.0.0 will be removed in 3.0.0. We may also noted that a deprecated element might be kept longer to ease migration but that developers should not rely on this. That way AsciidoctorJ users know that they need to remove deprecated code before upgrading to a major version.

@robertpanzer
Copy link
Member

Sorry if the comment about Scala was too harsh :)

Yes, that makes sense.
Removing deprecated elements in the next major release should be fine, so that we can remove these with 2.0.0.

@mojavelinux
Copy link
Member

My point was to define a rule regarding deprecated elements

These are the kinds of rules we're trying to establish right now to make it easier to manage the projects and so users know what to expect. I recommend starting by adding them to a version policy section in the README. We can then go through the repositories and collect these up to display somewhere on asciidoctor.org.

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

Successfully merging this pull request may close these issues.

3 participants