Skip to content

[SR-7293] Refactoring action to add Equatable Conformance #29847

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Apr 27, 2020
Merged

[SR-7293] Refactoring action to add Equatable Conformance #29847

merged 4 commits into from
Apr 27, 2020

Conversation

tkachukandrew
Copy link
Contributor

Add refactoring action to add Equatable conformance.

Resolves SR-7293.

@swift-ci Please smoke test

@tkachukandrew tkachukandrew changed the base branch from master to swift-5.2-branch February 14, 2020 17:18
@tkachukandrew tkachukandrew changed the base branch from swift-5.2-branch to master February 14, 2020 17:18
@theblixguy theblixguy requested a review from nathawes February 14, 2020 17:20
@tkachukandrew tkachukandrew force-pushed the add-equatable-conformance branch from 1925251 to ad15f21 Compare February 14, 2020 17:32
@CodaFi
Copy link
Contributor

CodaFi commented Apr 15, 2020

@swift-ci test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - ad15f21

@tkachukandrew
Copy link
Contributor Author

@CodaFi Could You please start tests one more time? Thanks in advance!

@owenv
Copy link
Contributor

owenv commented Apr 17, 2020

@swift-ci test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - ad15f21

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - ad15f21

@nathawes
Copy link
Contributor

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - fc382d2

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - fc382d2

@tkachukandrew
Copy link
Contributor Author

@nathawes Could You please review these changes one more time? All checks have passed. Thanks!

nathawes
nathawes requested changes Apr 22, 2020
View reviewed changes
Copy link
Contributor

@nathawes nathawes left a comment

Choose a reason for hiding this comment

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

This is looking great – thanks for implementing it, and sorry for the delay in reviewing!
Just left a few comments about a some extra test cases it would be good to add, and including non-public stored properties too to match the compiler's synthesized Equatable conformance behavior.

@tkachukandrew
Copy link
Contributor Author

tkachukandrew commented Apr 25, 2020 •
edited
Loading

Thanks @nathawes for reviewing! Sorry for a delay with committing changes.

Should I also create the same PR to release/5.3?

@tkachukandrew tkachukandrew requested a review from nathawes April 25, 2020 18:06
@tkachukandrew tkachukandrew force-pushed the add-equatable-conformance branch from 9d256c7 to d0ac023 Compare April 25, 2020 18:13
nathawes
nathawes approved these changes Apr 27, 2020
View reviewed changes
Copy link
Contributor

@nathawes nathawes left a comment

Choose a reason for hiding this comment

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

Thanks for adding the extra tests. This looks great!

@nathawes
Copy link
Contributor

@swift-ci test and merge

@tkachukandrew tkachukandrew mentioned this pull request Apr 27, 2020
Merged
@tkachukandrew
Copy link
Contributor Author

@nathawes Thanks for approving!
Looks like one check has failed due to Jenkins Error. Could you please restart build?

@nathawes
Copy link
Contributor

@swift-ci test and merge

@swift-ci swift-ci merged commit 041dfb9 into swiftlang:master Apr 27, 2020
@swift-ci swift-ci mentioned this pull request Feb 14, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nathawes nathawes nathawes approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tkachukandrew @CodaFi @swift-ci @owenv @nathawes