Skip to content

API: Extended some deprecation comments in API folder#5726

Merged
danielcweeks merged 1 commit intoapache:masterfrom
gaborkaszab:comment_deprecation
Sep 28, 2022
Merged

API: Extended some deprecation comments in API folder#5726
danielcweeks merged 1 commit intoapache:masterfrom
gaborkaszab:comment_deprecation

Conversation

@gaborkaszab
Copy link
Collaborator

There are some deprecation comments added after 0.14.0 in the API folder where there is no indication when the particular functionality is going to be dropped. I marked them to be dropped in 2.0.0.

@github-actions github-actions bot added the API label Sep 8, 2022
@gaborkaszab
Copy link
Collaborator Author

tagging @nastra @rdblue as the authors of the patches that marked these functions deprecated.

cc @danielcweeks as this came up in the #deprecations channel

@nastra
Copy link
Contributor

nastra commented Sep 8, 2022

This LGTM, @gaborkaszab can you please prefix your commit msg + PR title with API: xxx to indicate that this commit is for the api module

@XN137
Copy link
Contributor

XN137 commented Sep 8, 2022

@gaborkaszab please redo the commit, i dont see why i am listed as author here (merge/squash problem?)

private static final int DEFAULT_HISTOGRAM_RESERVOIR_SIZE = 10_000;

/**
* @deprecated will be removed in 2.0.0, use {@link org.apache.iceberg.metrics.Counter} instead.
Copy link
Member

Choose a reason for hiding this comment

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

Isn't 2.0.0 too far? Should we mention it as "will be removed after 1.0.0"?

Let's wait for @danielcweeks's response on this before addressing this.

Copy link
Contributor

Choose a reason for hiding this comment

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

2.0.0 is correct since we guarantee that we won't break these APIs in minor releases.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the clarification.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In theory you are only allowed to break the API in a major release bump, hence I've 2.0.0 here

There are some deprecation comments added after 0.14.0 in the API folder
where there is no indication when the particular functionality is going
to be dropped. I marked them to be dropped in 2.0.0.
@gaborkaszab gaborkaszab changed the title Extended some deprecation comments in API folder API: Extended some deprecation comments in API folder Sep 9, 2022
@gaborkaszab
Copy link
Collaborator Author

This LGTM, @gaborkaszab can you please prefix your commit msg + PR title with API: xxx to indicate that this commit is for the api module

Sure, added "API" prefix.

@gaborkaszab
Copy link
Collaborator Author

@gaborkaszab please redo the commit, i dont see why i am listed as author here (merge/squash problem?)

Oh, thanks for pointing this out! I had some misunderstandings with git before pushing the PR :)

@danielcweeks danielcweeks merged commit e8b972e into apache:master Sep 28, 2022
@gaborkaszab gaborkaszab deleted the comment_deprecation branch October 21, 2022 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants