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

Delete method definition in rails that is compatible with ruby defini… #36497

Merged
merged 1 commit into from Jun 16, 2019

Conversation

soartec-lab
Copy link
Contributor

The following methods are already defined in ruby's Date class and are compatible, so there is no need to extend Date class with rails.
It also proves that the test passes.
I moved it because I only needed the Time class.

Copy link
Member

@kamipo kamipo left a comment

Choose a reason for hiding this comment

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

Can you update the core extensions guide as well?

@rails-bot rails-bot bot added the docs label Jun 16, 2019
@soartec-lab
Copy link
Contributor Author

@kamipo
Thank you review.
The following commits have been added.

  • Correction of Doc pointed out by you
  • Testcase move
  • Update guide

If this is ok I will rebase the commit.
Please check it.

Copy link
Member

@kamipo kamipo left a comment

Choose a reason for hiding this comment

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

Looks good to me. Can you squash your commits into one?

guides/source/active_support_core_extensions.md Outdated Show resolved Hide resolved
guides/source/active_support_core_extensions.md Outdated Show resolved Hide resolved
guides/source/active_support_core_extensions.md Outdated Show resolved Hide resolved
guides/source/active_support_core_extensions.md Outdated Show resolved Hide resolved
guides/source/active_support_core_extensions.md Outdated Show resolved Hide resolved
guides/source/active_support_core_extensions.md Outdated Show resolved Hide resolved
guides/source/active_support_core_extensions.md Outdated Show resolved Hide resolved
@soartec-lab soartec-lab force-pushed the move_date_and_time_method_to_time branch from 14d40da to 2e5094d Compare June 16, 2019 09:10
…th ruby definition

Tests are also only on the `Time` class

Update doc forgetting to erase when moved

Update guide `Date` class to `Time` class and defined file

Update guide correction omission
@soartec-lab soartec-lab force-pushed the move_date_and_time_method_to_time branch from 2e5094d to bfecb7d Compare June 16, 2019 09:11
@soartec-lab
Copy link
Contributor Author

@kamipo

I corresponded to all the issues pointed out and squash commits into one.
If you like, please merge.

@kamipo kamipo merged commit 6a033a0 into rails:master Jun 16, 2019
@kamipo
Copy link
Member

kamipo commented Jun 16, 2019

Thanks!

By the way (it's off-topic), AFAICS you are Japanese (same with me), so I'd recommend double-checking with Japanese-English translation, it would help you hopefully.

image

@soartec-lab
Copy link
Contributor Author

Thank you for merging !

I'm still studying OSS development and English and I'm always bothering you. sorry.
However, I can learn many things by developing with you and it is very happy for me.
I look forward to working with you.

kamipo added a commit that referenced this pull request Jun 17, 2019
…to_time

Delete method definition in rails that is compatible with ruby defini…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants