4 ms·

I haven't seen your PRs and I don't work on those project. I have small projects that receive few patches. My experience of the few patches I have received tho

I haven't seen your PRs and I don't work on those project. I have small projects that receive few patches. My experience of the few patches I have received though is they are 100% without exception, bad patches. Bad in that, without me putting an hour or 2 of work into them I can't just accept them. The most common case is no tests. The patch fixes an issue, but the issue exists because there was no test for the case the patch is fixing. So, to accept the PR, I have to download it and spend time writing a test. Other common experiences are bad coding practices and non-matching styles so I have two choices (1) spend 30-60 minutes downloading the patch, fixing these issues myself (2) spend 40-60 minutes adding comments to try to get the person who posted the PR to make their patch acceptable (40-60 mins includes the back and forth). More often than not, (2) never gets a response. The contributor's POV is they provided a fix and I should be happy to take it as is. I get that. At a certain level they are correct. But, these projects are hobby projects and I have limited time. So I generally don't do (2) because if they ignore the comments then it's wasted time, and (1) has the hurdle that I need to take an hour out to deal with it.

xyst 9mo ago
Wouldn’t it be best to ask contributor to reproduce the bug with a test rather than just completely ghost them? Companies such as Google have billions at their disposal yet still can’t figure out simple project management? Google of the past is no more, unfortunately.
- pousada 9mo ago
  It’s not worth the time. You will spend uncountable hours of (unpaid) extremely exhausting labour talking to people who only care about solving their personal super specific problems. This is true for 90%+, there are exceptions but they are exceedingly rare Trust me I tried many many times. This has nothing to do with Google being evil it’s just one of the realities of maintaining a big open source project.
  - bruce511 9mo ago
    I'll add that for small projects, (and I suppose large ones) it's also a "unwelcome" task. Kinda like docs is. Open Source projects are typically done by people who like coding. Writing docs, reviewing PRs, "management" are all chores, not fun parts of the project. I manage a couple of projects that get submissions. Handling that is really not the fun part of my day. Fortunately I get very few. I can understand why ones that get a lot see it as a burden, not as the great gift the submitter thinks it is. Personally I don't want to spend hours each day reviewing PRs. That's not what I signed up for.
SOLAR_FIELDS 9mo ago
Your first example should be solved by the maintainers outlining clear contribution guidelines. It’s not hard to point some automation at a pr and comment if someone didn’t follow contribution guidelines. Nonmatching styles can be mostly solved with linting and static analysis There’s no fix for bad code outside of manual review beyond that. But doing those things should significantly cut down on your noise.
- pousada 9mo ago
  Trust me it’s not being solved by a CONTRIBUTING.md with guidelines. 90% of contributors won’t read it at all or only some parts of it and ignore the rest. Most PRs you get solve some super specific individual problem and aren’t relevant for the wider community that uses your OSS. It’s not their fault really but most contributions are so bad it doesn’t serve to spend any time on reviewing them earnestly. (Been maintaining several popular projects for the last 7 years)
  - judahmeek 9mo ago
    Can I get your opinion on https://github.com/Judahmeek/frogs https://github.com/Judahmeek/frogs as an open-source maintainer? Edit: https://judahmeek.com/p/we-need-frogs-to-defend-foss https://judahmeek.com/p/we-need-frogs-to-defend-foss may be the better link to start with.
    - pousada 9mo ago
      I don’t think there is a simple “fits-all” solution. In my case there is a monetised proprietary “enterprise” edition of the projects available. Contributions only get accepted if they fit into the commercial roadmap, which is shaped by the (paying) customer needs. It’s not perfect but the OSS “community” edition is still usable and valuable to many
      - judahmeek 9mo ago
        Thanks for taking the time to reply. I agree that there's no "fits-all" solution & I think that's a good sign of the open-source ecosystem's diversity.
mschuster91 9mo ago
One problem with tests is that every project has different philosophies on how to write and how to organize tests. Others have no way of running them locally because it‘s all CD, and often figuring out how to write and organize tests takes longer than fixing the bug itself. Or the stack has little support for running just one specific test (aka the test you‘re trying to write). Or tests need resources you don’t have.
lelanthran 9mo ago
That's funny :-) I say almost exactly the same thing about agent changes, but the impression I get from people heavily using agents is that they are plenty more flexible about what the code looks like than I am. I am starting to suspect that it is a personal failing of mine to require that all my code looks consistent within a single project.
- __turbobrew__ 9mo ago
  Well run projects I have contributed to have linters which fail on bad code style. Ask the submitter to make the linter happy before you review the code.
  - lelanthran 9mo ago
    > Well run projects I have contributed to have linters which fail on bad code style. Ask the submitter to make the linter happy before you review the code. Linters can't catch most things that are not syntax-style; i.e. linters can't catch semantic style. Here is code CC generated this morning for me: size_t bvks_tls_read (bvks_tls_t *tls, void *dst, size_t dst_len) { int ret = wolfSSL_read (tls->ssl, dst, (int)dst_len); if (ret > 0) return (size_t)ret; if (ret == 0) return 0; // ret < 0, error case int err = wolfSSL_get_error (tls->ssl, ret); switch (err) { case WOLFSSL_ERROR_WANT_READ: case WOLFSSL_ERROR_WANT_WRITE: errno = EAGAIN; break; case WOLFSSL_ERROR_SYSCALL: // errno already set by the underlying socket call break; default: errno = EPROTO; break; } return (size_t)-1; } size_t bvks_tls_write (bvks_tls_t *tls, const void *src, size_t src_len) { int ret = wolfSSL_write (tls->ssl, src, (int)src_len); if (ret > 0) return (size_t)ret; if (ret == 0) return 0; // ret < 0, error case int err = wolfSSL_get_error (tls->ssl, ret); switch (err) { case WOLFSSL_ERROR_WANT_READ: case WOLFSSL_ERROR_WANT_WRITE: errno = EAGAIN; break; case WOLFSSL_ERROR_SYSCALL: // errno already set by the underlying socket call break; default: errno = EPROTO; break; } return (size_t)-1; } Here is my preferred style: typedef int (tls_op_func_t) (WOLFSSL *, void *, int); static size_t tls_op (WOLFSSL *ssl, tls_op_func_t *fptr, void *buf, size_t len) { if (len > INT_MAX) { errno = EINVAL; return (size_t)-1; } int wbytes = fptr (ssl, buf, (int)len); if (wbytes >= 0) return (size_t)wbytes; errno = map_wolf_error (wolfSSL_get_error (ssl, wbytes)); return (size_t)-1; } size_t bvks_tls_read (bvks_tls_t *tls, void *dst, size_t dst_len) { return tls_op (tls->ssl, wolfSSL_read, dst, dst_len); } size_t bvks_tls_write (bvks_tls_t *tls, const void *src, size_t src_len) { return tls_op (tls->ssl, (tls_op_func_t *)wolfSSL_write, (void *)src, src_len); } One of those is both hard to maintain and has precision (and potential overflow) bugs. The other isolates the potentially buggy behaviour, validates it, and ensures that future changes to fix size_t/int precision loss bugs only has to be done in a single spot. No linter is catching that style. It's more "coding style" than "syntax style".
__turbobrew__ 9mo ago
It isn’t a me problem, I have had no issues upstreaming patches to other projects which have responsive maintainers but etcd and k8s just seem to be straight up mismanaged. I read contributors.md, I make sure all the linters pass, I make sure all the tests pass, I sign off the commits with my real identity to fulfill the CLA. Yes there is a lot of garbage out there, but for the people who are actually trying to fix issues it is impossible without an insider within the project.
- kwdev 9mo ago
  Feel free to link your PRs, happy to take a look.
  - __turbobrew__ 9mo ago
    As said in other comment, my PRs are using my real name and my HN account does not, so I don’t want to connect the two by linking my PRs.
__s 9mo ago
https://github.com/google/flatbuffers/pull/8251 https://github.com/google/flatbuffers/pull/8251 https://github.com/google/flatbuffers/pull/8252 https://github.com/google/flatbuffers/pull/8252 Flatbuffers Lua implementation has basic fundamental flaws, couldn't even get an "idc" on their Discord

xyst 9mo ago

Wouldn’t it be best to ask contributor to reproduce the bug with a test rather than just completely ghost them? Companies such as Google have billions at their disposal yet still can’t figure out simple project management? Google of the past is no more, unfortunately.

pousada 9mo ago
It’s not worth the time. You will spend uncountable hours of (unpaid) extremely exhausting labour talking to people who only care about solving their personal super specific problems. This is true for 90%+, there are exceptions but they are exceedingly rare Trust me I tried many many times. This has nothing to do with Google being evil it’s just one of the realities of maintaining a big open source project.
- bruce511 9mo ago
  I'll add that for small projects, (and I suppose large ones) it's also a "unwelcome" task. Kinda like docs is. Open Source projects are typically done by people who like coding. Writing docs, reviewing PRs, "management" are all chores, not fun parts of the project. I manage a couple of projects that get submissions. Handling that is really not the fun part of my day. Fortunately I get very few. I can understand why ones that get a lot see it as a burden, not as the great gift the submitter thinks it is. Personally I don't want to spend hours each day reviewing PRs. That's not what I signed up for.

SOLAR_FIELDS 9mo ago

Your first example should be solved by the maintainers outlining clear contribution guidelines. It’s not hard to point some automation at a pr and comment if someone didn’t follow contribution guidelines. Nonmatching styles can be mostly solved with linting and static analysis There’s no fix for bad code outside of manual review beyond that. But doing those things should significantly cut down on your noise.

pousada 9mo ago
Trust me it’s not being solved by a CONTRIBUTING.md with guidelines. 90% of contributors won’t read it at all or only some parts of it and ignore the rest. Most PRs you get solve some super specific individual problem and aren’t relevant for the wider community that uses your OSS. It’s not their fault really but most contributions are so bad it doesn’t serve to spend any time on reviewing them earnestly. (Been maintaining several popular projects for the last 7 years)
- judahmeek 9mo ago
  Can I get your opinion on https://github.com/Judahmeek/frogs https://github.com/Judahmeek/frogs as an open-source maintainer? Edit: https://judahmeek.com/p/we-need-frogs-to-defend-foss https://judahmeek.com/p/we-need-frogs-to-defend-foss may be the better link to start with.
  - pousada 9mo ago
    I don’t think there is a simple “fits-all” solution. In my case there is a monetised proprietary “enterprise” edition of the projects available. Contributions only get accepted if they fit into the commercial roadmap, which is shaped by the (paying) customer needs. It’s not perfect but the OSS “community” edition is still usable and valuable to many
    - judahmeek 9mo ago
      Thanks for taking the time to reply. I agree that there's no "fits-all" solution & I think that's a good sign of the open-source ecosystem's diversity.

mschuster91 9mo ago

One problem with tests is that every project has different philosophies on how to write and how to organize tests. Others have no way of running them locally because it‘s all CD, and often figuring out how to write and organize tests takes longer than fixing the bug itself. Or the stack has little support for running just one specific test (aka the test you‘re trying to write). Or tests need resources you don’t have.

lelanthran 9mo ago

That's funny :-) I say almost exactly the same thing about agent changes, but the impression I get from people heavily using agents is that they are plenty more flexible about what the code looks like than I am. I am starting to suspect that it is a personal failing of mine to require that all my code looks consistent within a single project.

__turbobrew__ 9mo ago
Well run projects I have contributed to have linters which fail on bad code style. Ask the submitter to make the linter happy before you review the code.
- lelanthran 9mo ago
  > Well run projects I have contributed to have linters which fail on bad code style. Ask the submitter to make the linter happy before you review the code. Linters can't catch most things that are not syntax-style; i.e. linters can't catch semantic style. Here is code CC generated this morning for me: size_t bvks_tls_read (bvks_tls_t *tls, void *dst, size_t dst_len) { int ret = wolfSSL_read (tls->ssl, dst, (int)dst_len); if (ret > 0) return (size_t)ret; if (ret == 0) return 0; // ret < 0, error case int err = wolfSSL_get_error (tls->ssl, ret); switch (err) { case WOLFSSL_ERROR_WANT_READ: case WOLFSSL_ERROR_WANT_WRITE: errno = EAGAIN; break; case WOLFSSL_ERROR_SYSCALL: // errno already set by the underlying socket call break; default: errno = EPROTO; break; } return (size_t)-1; } size_t bvks_tls_write (bvks_tls_t *tls, const void *src, size_t src_len) { int ret = wolfSSL_write (tls->ssl, src, (int)src_len); if (ret > 0) return (size_t)ret; if (ret == 0) return 0; // ret < 0, error case int err = wolfSSL_get_error (tls->ssl, ret); switch (err) { case WOLFSSL_ERROR_WANT_READ: case WOLFSSL_ERROR_WANT_WRITE: errno = EAGAIN; break; case WOLFSSL_ERROR_SYSCALL: // errno already set by the underlying socket call break; default: errno = EPROTO; break; } return (size_t)-1; } Here is my preferred style: typedef int (tls_op_func_t) (WOLFSSL *, void *, int); static size_t tls_op (WOLFSSL *ssl, tls_op_func_t *fptr, void *buf, size_t len) { if (len > INT_MAX) { errno = EINVAL; return (size_t)-1; } int wbytes = fptr (ssl, buf, (int)len); if (wbytes >= 0) return (size_t)wbytes; errno = map_wolf_error (wolfSSL_get_error (ssl, wbytes)); return (size_t)-1; } size_t bvks_tls_read (bvks_tls_t *tls, void *dst, size_t dst_len) { return tls_op (tls->ssl, wolfSSL_read, dst, dst_len); } size_t bvks_tls_write (bvks_tls_t *tls, const void *src, size_t src_len) { return tls_op (tls->ssl, (tls_op_func_t *)wolfSSL_write, (void *)src, src_len); } One of those is both hard to maintain and has precision (and potential overflow) bugs. The other isolates the potentially buggy behaviour, validates it, and ensures that future changes to fix size_t/int precision loss bugs only has to be done in a single spot. No linter is catching that style. It's more "coding style" than "syntax style".

__turbobrew__ 9mo ago

It isn’t a me problem, I have had no issues upstreaming patches to other projects which have responsive maintainers but etcd and k8s just seem to be straight up mismanaged. I read contributors.md, I make sure all the linters pass, I make sure all the tests pass, I sign off the commits with my real identity to fulfill the CLA. Yes there is a lot of garbage out there, but for the people who are actually trying to fix issues it is impossible without an insider within the project.

kwdev 9mo ago
Feel free to link your PRs, happy to take a look.
- __turbobrew__ 9mo ago
  As said in other comment, my PRs are using my real name and my HN account does not, so I don’t want to connect the two by linking my PRs.

__s 9mo ago

https://github.com/google/flatbuffers/pull/8251 https://github.com/google/flatbuffers/pull/8251 https://github.com/google/flatbuffers/pull/8252 https://github.com/google/flatbuffers/pull/8252 Flatbuffers Lua implementation has basic fundamental flaws, couldn't even get an "idc" on their Discord