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

Remove deprecated UsersLookupByEmailResponse parent class (deprecated in v1.1) #551

Closed

Conversation

seratch
Copy link
Member

@seratch seratch commented Sep 2, 2020

This pull request removes a class kept for backward-compatibility since v1.1.0. I'm sure this is safe enough as the class has been marked as deprecated in v1.1 plus it's not a runtime error but a compilation failure.

Related to #549 #486

Category (place an x in each of the [ ])

  • bolt (Bolt for Java)
  • bolt-{sub modules} (Bolt for Java - optional modules)
  • slack-api-client (Slack API Clients)
  • slack-api-model (Slack API Data Models)
  • slack-api-*-kotlin-extension (Kotlin Extensions for Slack API Clients)
  • slack-app-backend (The primitive layer of Bolt for Java)

Requirements

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

@seratch seratch added the project:slack-api-client project:slack-api-client label Sep 2, 2020
@seratch seratch added this to the 1.2.0 milestone Sep 2, 2020
@seratch seratch self-assigned this Sep 2, 2020
@codecov
Copy link

codecov bot commented Sep 2, 2020

Codecov Report

Merging #551 into main will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #551      +/-   ##
============================================
+ Coverage     83.75%   83.81%   +0.05%     
  Complexity     2694     2694              
============================================
  Files           297      296       -1     
  Lines          7191     7190       -1     
  Branches        611      611              
============================================
+ Hits           6023     6026       +3     
+ Misses          803      799       -4     
  Partials        365      365              
Impacted Files Coverage Δ Complexity Δ
...pi/methods/metrics/impl/RedisMetricsDatastore.java 89.50% <0.00%> (+2.46%) 38.00% <0.00%> (+1.00%)

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 c5804ce...1300d80. Read the comment docs.

@seratch
Copy link
Member Author

seratch commented Sep 2, 2020

We will merge this after v1.1.4 release.

@mwbrooks mwbrooks self-requested a review September 2, 2020 18:55
Copy link
Member

@mwbrooks mwbrooks left a comment

Choose a reason for hiding this comment

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

@seratch I noticed that this is part of the 1.2.0 milestone but removing the deprecated class will cause a breaking change (compile error). Following semver rules, should this be a major (2.0.0 release)?

@mwbrooks mwbrooks added the enhancement M-T: A feature request for new functionality label Sep 2, 2020
@seratch
Copy link
Member Author

seratch commented Sep 2, 2020

Following semver rules, should this be a major (2.0.0 release)?

I think this is a very minor breaking change but, yes, strictly speaking, you're right. I agree we should follow the rule.

Let me close this PR and will do this in v2.0.

@seratch seratch closed this Sep 2, 2020
@seratch seratch modified the milestones: 1.2.0, 2.0.0 Sep 2, 2020
@seratch seratch deleted the issue-549-remove-deprecated-class branch October 13, 2020 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement M-T: A feature request for new functionality project:slack-api-client project:slack-api-client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants