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 unnecessary argument to Map.sub #3275

Merged
merged 1 commit into from
Aug 14, 2019

Conversation

rkallos
Copy link
Contributor

@rkallos rkallos commented Aug 7, 2019

Map.sub accepts a value argument that it never uses.

@rkallos rkallos force-pushed the fix/unnecessary-arg-to-map-sub branch from dd22a5f to 8d81bc4 Compare August 7, 2019 02:27
@SeanTAllen
Copy link
Member

Can you add release notes for this breaking change to #3274, including before and after code for the change that folks need to make.

Also, I think this should be under Changed not Fixed in the changelog.

Copy link
Member

@SeanTAllen SeanTAllen left a comment

Choose a reason for hiding this comment

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

Changelog entry should be under Changed rather than Fixed.

@rkallos rkallos force-pushed the fix/unnecessary-arg-to-map-sub branch from 8d81bc4 to 425154d Compare August 7, 2019 02:44
@rkallos rkallos force-pushed the fix/unnecessary-arg-to-map-sub branch from 425154d to b4b96cb Compare August 7, 2019 02:45
@rkallos rkallos changed the title Remove unnecessary argument to Map.sub Remove unnecessary argument to Map.sub, add add,sub to persistent/Map Aug 7, 2019
Copy link
Member

@SeanTAllen SeanTAllen left a comment

Choose a reason for hiding this comment

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

Nevermind I see this is two different commits.

@SeanTAllen
Copy link
Member

Do not squash the two commits in this PR when merging.

@rkallos rkallos force-pushed the fix/unnecessary-arg-to-map-sub branch from b4b96cb to 32e449d Compare August 7, 2019 02:53
@rkallos rkallos changed the title Remove unnecessary argument to Map.sub, add add,sub to persistent/Map Remove unnecessary argument to Map.sub Aug 13, 2019
@aturley
Copy link
Member

aturley commented Aug 13, 2019

@SeanTAllen do you have any other concerns on this or can it be merged?

@SeanTAllen
Copy link
Member

@aturley yes, I'm good

@rkallos
Copy link
Contributor Author

rkallos commented Aug 13, 2019

#3274 (comment)

@rkallos rkallos requested a review from SeanTAllen August 13, 2019 16:31
@rkallos rkallos merged commit 740e28f into ponylang:master Aug 14, 2019
@rkallos rkallos deleted the fix/unnecessary-arg-to-map-sub branch August 14, 2019 20:45
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.

4 participants