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

Fix kwargs for Ruby 2.7 #885

Merged
merged 2 commits into from
Jan 27, 2021
Merged

Conversation

oieioi
Copy link
Contributor

@oieioi oieioi commented May 9, 2020

Description

Fix kwargs for Ruby 2.7.

References

@brian-kephart
Copy link

Apologies, I had the wrong branch checked out when I tested this PR and requested changes. The PR works, and I deleted my earlier comment. Thanks @oieioi!

brian-kephart
brian-kephart previously approved these changes Dec 28, 2020
@codebycliff
Copy link
Collaborator

This looks good. I apologize for how long it has taken me to getting around to this. Do you want to rebase / update the branch with master? I can also update the branch directly if you are comfortable with that.

@ojab
Copy link

ojab commented Jan 14, 2021

Probably 3.0 should be added to CI here since it's out and fixing kwargs for 2.7 is not that required per se.

@oieioi
Copy link
Contributor Author

oieioi commented Jan 15, 2021

@codebycliff I merged master branch, should I rebase it?

@n-rodriguez
Copy link
Contributor

@oieioi can you please rebase your branch on master?

@oieioi
Copy link
Contributor Author

oieioi commented Jan 22, 2021

@n-rodriguez Yes. I rebased and force-pushed it.

@n-rodriguez
Copy link
Contributor

@oieioi thank you !

@n-rodriguez
Copy link
Contributor

n-rodriguez commented Jan 26, 2021

@oieioi I've added a commit on top of yours to add Ruby 3 CI : oieioi#1

All is green https://github.com/n-rodriguez/draper/runs/1769538026?check_suite_focus=true 👍

ping @codebycliff

@codebycliff codebycliff mentioned this pull request Jan 26, 2021
2 tasks
@codebycliff
Copy link
Collaborator

Also, I think we are good on this, but can someone double-check that this pull request isn't going to cause compatibility issues: https://github.com/drapergem/draper/pull/870/files?

@n-rodriguez
Copy link
Contributor

n-rodriguez commented Jan 26, 2021

Also, I think we are good on this, but can someone double-check that this pull request isn't going to cause compatibility issues: https://github.com/drapergem/draper/pull/870/files?

Since #870 is already in master it has been covered by https://github.com/n-rodriguez/draper/runs/1769538026?check_suite_focus=true

From what I see it looks good to me.

@oieioi
Copy link
Contributor Author

oieioi commented Jan 27, 2021

@n-rodriguez @codebycliff Thank you for oieioi#1. I merged it.

Copy link
Collaborator

@codebycliff codebycliff left a comment

Choose a reason for hiding this comment

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

Thanks everyone! Sorry for the delay on all this.

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.

5 participants