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

add memoising for layer info #52

Merged
merged 5 commits into from
Mar 14, 2022
Merged

add memoising for layer info #52

merged 5 commits into from
Mar 14, 2022

Conversation

maelle
Copy link
Collaborator

@maelle maelle commented Mar 2, 2022

Fix #50

(so no need for an article in the end :-) )

R/info.R Outdated Show resolved Hide resolved
@annakrystalli
Copy link
Collaborator

Awesome!

I'd like to start merging some PRs. Maybe this is a good one to start with if at least the mac & windows tests pass?

As this is user facing, should we add it to NEWS.md and bump the version?

P.S. I royally messed up semantic versioning with this 😭 I know

@maelle
Copy link
Collaborator Author

maelle commented Mar 2, 2022

Should we use fledge? https://cynkra.github.io/fledge/

@annakrystalli
Copy link
Collaborator

Fledge looks super interesting! I'm tempted to implement alongside a CONTRIBUTING.md to ensure I am fully familiar with it's use and it's documented.

Let's open an issue about it for now, Is that cool?

@maelle
Copy link
Collaborator Author

maelle commented Mar 2, 2022

Added a news item and opened an issue :-)

Copy link
Collaborator

@annakrystalli annakrystalli left a comment

Choose a reason for hiding this comment

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

LGTM. Maybe worth rerunning the tests?

@maelle maelle merged commit 728329d into master Mar 14, 2022
@maelle maelle deleted the memoise branch March 14, 2022 13:26
This pull request was closed.
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.

Create an overview of all the data?
2 participants