Skip to content

Conversation

@erlend-aasland
Copy link
Contributor

@erlend-aaslanderlend-aasland commented Jan 6, 2021

@erlend-aasland
Copy link
ContributorAuthor

skip news

@erlend-aasland
Copy link
ContributorAuthor

FYI, rebased onto master bco. #20828

@erlend-aasland
Copy link
ContributorAuthor

erlend-aasland commented Jan 7, 2021

@berkerpeksag What about ditching the whole Cache/Node stuff and just use functools.lru_cache? Pro: It would get rid of a lot of code. Con: It might be a little bit slower. Worth pursuing as a future enhancement?

@erlend-aasland
Copy link
ContributorAuthor

@serhiy-storchaka Would you mind reviewing this?

@berkerpeksag
Copy link
Member

@berkerpeksag What about ditching the whole Cache/Node stuff and just use functools.lru_cache?

I'm usually not a fan of changing code that has been working fine for more than a decade, but let me think about it a bit.

I'm going to review this PR this week.

@erlend-aasland
Copy link
ContributorAuthor

erlend-aasland commented Jan 11, 2021

I'm usually not a fan of changing code that has been working fine for more than a decade, but let me think about it a bit.

That's a valid argument, but take a look at my arguments at bpo-42862.

I'm going to review this PR this week.

Great, thanks.

@erlend-aasland
Copy link
ContributorAuthor

Thanks, @serhiy-storchaka 🙏🏻

@erlend-aasland
Copy link
ContributorAuthor

Closing this, as it has been made obsolete by #24203.

@erlend-aaslanderlend-aasland deleted the bpo-40956/part5-cache branch February 2, 2021 20:49
Sign up for freeto 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.

5 participants

@erlend-aasland@berkerpeksag@serhiy-storchaka@the-knights-who-say-ni@bedevere-bot