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.

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

Add .memoized? method #17

Merged
merged 2 commits into from
Oct 1, 2019
Merged

Add .memoized? method #17

merged 2 commits into from
Oct 1, 2019

Conversation

AlexWayfer
Copy link
Contributor

To check (externally) if method memoized or not.

@AlexWayfer AlexWayfer changed the title Add .memoized? method WIP: Add .memoized? method Sep 19, 2019
@AlexWayfer AlexWayfer changed the title WIP: Add .memoized? method Add .memoized? method Sep 19, 2019
tycooon
tycooon reviewed Sep 20, 2019
View reviewed changes
spec/memery_spec.rb Outdated Show resolved Hide resolved
@tycooon
Copy link
Owner

tycooon commented Sep 20, 2019

Please add new method to the Readme.

@coveralls
Copy link

coveralls commented Sep 24, 2019 •
edited
Loading

Pull Request Test Coverage Report for Build 59

  • 37 of 41 (90.24%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-1.7%) to 98.259%
Changes Missing Coverage Covered Lines Changed/Added Lines %
spec/memery_spec.rb 35 39 89.74%
Totals Coverage Status
Change from base Build 54: -1.7%
Covered Lines: 903
Relevant Lines: 919
💛 - Coveralls

@AlexWayfer
Copy link
Contributor Author

Please add new method to the Readme.

Yes, done.

tycooon
tycooon reviewed Sep 24, 2019
View reviewed changes
lib/memery.rb Outdated Show resolved Hide resolved
Repository owner deleted a comment from coveralls Sep 25, 2019
@tycooon
Copy link
Owner

tycooon commented Sep 25, 2019

Please check the coverage, it looks like there is some dead code in specs: https://coveralls.io/builds/25906116

@AlexWayfer
Copy link
Contributor Author

Please check the coverage, it looks like there is some dead code in specs

I think we should filter /spec from coverage, but OK, I've removed the body of unused method.

@tycooon
Copy link
Owner

tycooon commented Sep 26, 2019 •
edited
Loading

Please check the coverage, it looks like there is some dead code in specs

I think we should filter /spec from coverage, but OK, I've removed the body of unused method.

It still shows coverage decrease. Why do we need the code that is never called in specs? 🤔

@AlexWayfer
Add .memoized? method
f2f12bf 
To check (externally) if method memoized or not.
@AlexWayfer
Copy link
Contributor Author

It still shows coverage decrease. Why do we need the code that is never called in specs? thinking

OK, another one, I'm sorry. Removed.

tycooon
tycooon requested changes Sep 28, 2019
View reviewed changes
lib/memery.rb Show resolved Hide resolved
@tycooon tycooon merged commit 9197d25 into tycooon:master Oct 1, 2019
@AlexWayfer AlexWayfer deleted the add_memoized_method branch October 27, 2020 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tycooon tycooon tycooon 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.
3 participants
@AlexWayfer @tycooon @coveralls