Skip to content

all: use t.TempDir() instead of os.TempDir() in tests#30168

Closed
lfz941 wants to merge 2 commits into
ethereum:masterfrom
lfz941:chore/test-tempdir
Closed

all: use t.TempDir() instead of os.TempDir() in tests#30168
lfz941 wants to merge 2 commits into
ethereum:masterfrom
lfz941:chore/test-tempdir

Conversation

@lfz941
Copy link
Copy Markdown

@lfz941 lfz941 commented Jul 16, 2024

Similar to #30150

@lfz941 lfz941 marked this pull request as draft July 16, 2024 11:18
@lfz941 lfz941 changed the title chore(test): using t.TempDir() instead of os.TempDir() in test (WIP)chore(test): using t.TempDir() instead of os.TempDir() in test Jul 16, 2024
@fjl fjl changed the title (WIP)chore(test): using t.TempDir() instead of os.TempDir() in test all: use t.TempDir() instead of os.TempDir() in tests Jul 16, 2024
@lightclient
Copy link
Copy Markdown
Member

From @karalabe:

In the past many months we have had a lot of issues with people opening PRs that are - for the most part - not useful. These include typo fixes, performance optimisations in non-hot code paths, variable renames, error message tweaks, etc. Whilst these PRs aren't unilaterally bad or worthless, most people are not opening them to make our codebase better, rather they are opening to farm commits for airdrops.

From our perspective, these commits often take a non-negligible effort to review and merge, because they keep touching sensitive code, that we need to be extra careful to understand. Even trivial fixes require attention and keep distracting us from focusing on developing and maintaining Geth.

We've decided to be more agressive in rejecting outside contributions that do not make a meaningful improvement:

  • Issues flagged by linters or third party tools should be discussed on a GitHub issue first to see if it makes sense to fix them, and if so, the correct way is to add a new linter step to our CIs so that issues are caught when being added, not kept being fixed in perpetuity.
  • Optimisations (especially when they lower code readability) should be done only in hot paths where performance actually matters, and even in those cases, there should be some benchmarks (ideally from the existing ones) highlighting their benefit.
  • As for typos, variable renames, error rewordings and such, should be opened as part of larger meaningful changes, not as tiny standalone PRs.

We appreciate the enthusiasm and effort of opening a PR against Geth, but due to the significant abuse against our time, we'll be closing PRs of the above type for the foreseeable future. We apologise if someone's genuine effort gets caught up in this agressive stance.

@lightclient lightclient closed this Aug 2, 2024
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.

2 participants