Feedback requested on: SOLR-14014: Allow Solr to start with Admin UI disabled
Please respond on the ticket: https://issues.apache.org/jira/browse/SOLR-14014 This is in response to discussions SOLR-13987: fix admin UI to not rely on javascript eval() which is a security issue. https://issues.apache.org/jira/browse/SOLR-13987 SOLR-14014 is not the final solution. It mitigates the issues discussed in SOLR-13987 in the short term until a final solution is determined. Joel Bernstein http://joelsolr.blogspot.com/
Re: Commit / Code Review Policy
And a link to guidelines on what goes where On Thu, Dec 5, 2019 at 10:49 AM Jan Høydahl wrote: > The SIP template should have a question that each proposal MUST answer: > > “Describe your consideration of what goes in solr-core and what goes in > packages or contrib.” > > Jan Høydahl > > 5. des. 2019 kl. 14:22 skrev Robert Muir : > > > Fine, here's my SIP process. > > You can't add one feature unless you remove two others. > > Very simple and works. > > On Thu, Dec 5, 2019 at 4:11 AM Jan Høydahl wrote: > >> Let’s keep this thread about code review guidelines, not about feature >> removal, please. And be concrete with proposals for re-wording Davids >> proposal instead of sidetracking. >> As David said, we need to do both. I think the SIP process for new >> features and APIs may be a far better way than some hard «feature freeze». >> >> Jan >> >> > 4. des. 2019 kl. 22:49 skrev Noble Paul : >> > >> >> I don't think this has anything to do with code review: it has to do >> with people just piling in features, but not taking the time to do any >> janitorial work or remove old features that shouldn't be there anymore (I >> AM LOOKING AT YOU HDFS) >> > >> > 100 %. If there is one problem with Solr today, it is feature bloat. >> > We need to trim down Solr by atleast 50% >> > >> > What we need to do urgently is to create a white list of essential >> > features and eliminate others. We are just getting crushed by the >> > amount of code we have in Solr. We don't have so many people who can >> > actively maintain such a large codebase >> > >> > On Thu, Dec 5, 2019 at 7:34 AM David Smiley >> wrote: >> >> >> >> Mike D., >> >> I loved your response, especially for researching what other projects >> do! >> >> >> >> ... more responses within... >> >> >> >> On Tue, Dec 3, 2019 at 2:42 PM Mike Drob wrote: >> >>> >> >>> I'm coming late into this thread because a lot of the discussion >> happened while I was offline, so some of the focus has shifted, but I'll >> try to address a few topics that were brought up earlier anyway. In an >> effort to be brief, I'll try to summarize sentiments rather than addressing >> specific quotes - if I mischaracterize something please let me know! >> >>> >> >>> First, I don't believe that RTC requires consensus voting with 3 >> reviews per commit or anything nearly so burdensome. A brief survey of >> other Apache projects shows that most on RTC only need a single review, and >> also can include exceptions for trivial changes like we are discussing >> here. If we're trying to find a compromise position, I personally would >> prefer to be more on the RTC side because I think it's a more responsible >> place to be for a project that backs billions of dollars of revenue across >> hundreds of companies annually. >> >>> >> >>> Spark is pretty strict RTC, but with such a volume of contributions >> it may be the only option sustainable for them. >> https://spark.apache.org/contributing.html >> >>> HBase requires a review and has an exception for trivial patches - >> http://hbase.apache.org/book.html#_review >> >>> Yetus requires reviews, but allows binding review from >> non-committers, and has a no review expiration. >> https://s.apache.org/mi0r8 - there's a lot of other good discussion >> there too. >> >>> Zookeeper requires a minimum one day waiting period on all code >> change, but does not require explicit reviews. - >> https://zookeeper.apache.org/bylaws.html >> >>> >> >>> One piece I'll echo from the Yetus discussion is that if we have a >> habit of review, then we're less likely to get stagnant patches, and we're >> also more likely to engage with non-committer contributions. If we don't >> have that habit of reviews, then patches do get stale, and >> trust/self-enforcement becomes the only sustainable way forward. >> >>> >> >>> A second point that I'm also concerned about is the idea that we >> don't want JIRA issues or CHANGES entries for trivial or even minor >> patches. This has a huge impact on potential contributors and their >> accessibility to the project. If contributors aren't credited for all of >> their work, then it makes it harder for them to reach invitation as a >> committer. As a personal example, a lot of my changes were around logging >> and error handling, which are minor on your list but fairly important for a >> supporter in aggregate. If the community signalled that the work was less >> valuable, less visible, and less likely to be accepted (each of which can >> follow from the previous I think) then I don't know that I would have been >> motivated to try and contribute those issues. >> >> >> >> >> >> Our CHANGES.txt tries to simultaneously be a useful document in >> informing users (and us) of what was changed for what issue that we might >> actually care about, and *also* give kudos to contributors. There is a lot >> of noise in there, as it is. If hypothetically a contributor files a JIRA >> issue with minor/trivial priority, then maybe a git author
Re: Commit / Code Review Policy
The SIP template should have a question that each proposal MUST answer: “Describe your consideration of what goes in solr-core and what goes in packages or contrib.” Jan Høydahl > 5. des. 2019 kl. 14:22 skrev Robert Muir : > > > Fine, here's my SIP process. > > You can't add one feature unless you remove two others. > > Very simple and works. > >> On Thu, Dec 5, 2019 at 4:11 AM Jan Høydahl wrote: >> Let’s keep this thread about code review guidelines, not about feature >> removal, please. And be concrete with proposals for re-wording Davids >> proposal instead of sidetracking. >> As David said, we need to do both. I think the SIP process for new features >> and APIs may be a far better way than some hard «feature freeze». >> >> Jan >> >> > 4. des. 2019 kl. 22:49 skrev Noble Paul : >> > >> >> I don't think this has anything to do with code review: it has to do with >> >> people just piling in features, but not taking the time to do any >> >> janitorial work or remove old features that shouldn't be there anymore (I >> >> AM LOOKING AT YOU HDFS) >> > >> > 100 %. If there is one problem with Solr today, it is feature bloat. >> > We need to trim down Solr by atleast 50% >> > >> > What we need to do urgently is to create a white list of essential >> > features and eliminate others. We are just getting crushed by the >> > amount of code we have in Solr. We don't have so many people who can >> > actively maintain such a large codebase >> > >> > On Thu, Dec 5, 2019 at 7:34 AM David Smiley >> > wrote: >> >> >> >> Mike D., >> >> I loved your response, especially for researching what other projects do! >> >> >> >> ... more responses within... >> >> >> >> On Tue, Dec 3, 2019 at 2:42 PM Mike Drob wrote: >> >>> >> >>> I'm coming late into this thread because a lot of the discussion >> >>> happened while I was offline, so some of the focus has shifted, but I'll >> >>> try to address a few topics that were brought up earlier anyway. In an >> >>> effort to be brief, I'll try to summarize sentiments rather than >> >>> addressing specific quotes - if I mischaracterize something please let >> >>> me know! >> >>> >> >>> First, I don't believe that RTC requires consensus voting with 3 reviews >> >>> per commit or anything nearly so burdensome. A brief survey of other >> >>> Apache projects shows that most on RTC only need a single review, and >> >>> also can include exceptions for trivial changes like we are discussing >> >>> here. If we're trying to find a compromise position, I personally would >> >>> prefer to be more on the RTC side because I think it's a more >> >>> responsible place to be for a project that backs billions of dollars of >> >>> revenue across hundreds of companies annually. >> >>> >> >>> Spark is pretty strict RTC, but with such a volume of contributions it >> >>> may be the only option sustainable for them. >> >>> https://spark.apache.org/contributing.html >> >>> HBase requires a review and has an exception for trivial patches - >> >>> http://hbase.apache.org/book.html#_review >> >>> Yetus requires reviews, but allows binding review from non-committers, >> >>> and has a no review expiration. https://s.apache.org/mi0r8 - there's a >> >>> lot of other good discussion there too. >> >>> Zookeeper requires a minimum one day waiting period on all code change, >> >>> but does not require explicit reviews. - >> >>> https://zookeeper.apache.org/bylaws.html >> >>> >> >>> One piece I'll echo from the Yetus discussion is that if we have a habit >> >>> of review, then we're less likely to get stagnant patches, and we're >> >>> also more likely to engage with non-committer contributions. If we don't >> >>> have that habit of reviews, then patches do get stale, and >> >>> trust/self-enforcement becomes the only sustainable way forward. >> >>> >> >>> A second point that I'm also concerned about is the idea that we don't >> >>> want JIRA issues or CHANGES entries for trivial or even minor patches. >> >>> This has a huge impact on potential contributors and their accessibility >> >>> to the project. If contributors aren't credited for all of their work, >> >>> then it makes it harder for them to reach invitation as a committer. As >> >>> a personal example, a lot of my changes were around logging and error >> >>> handling, which are minor on your list but fairly important for a >> >>> supporter in aggregate. If the community signalled that the work was >> >>> less valuable, less visible, and less likely to be accepted (each of >> >>> which can follow from the previous I think) then I don't know that I >> >>> would have been motivated to try and contribute those issues. >> >> >> >> >> >> Our CHANGES.txt tries to simultaneously be a useful document in informing >> >> users (and us) of what was changed for what issue that we might actually >> >> care about, and *also* give kudos to contributors. There is a lot of >> >> noise in there, as it is. If
Re: Commit / Code Review Policy
Fine, here's my SIP process. You can't add one feature unless you remove two others. Very simple and works. On Thu, Dec 5, 2019 at 4:11 AM Jan Høydahl wrote: > Let’s keep this thread about code review guidelines, not about feature > removal, please. And be concrete with proposals for re-wording Davids > proposal instead of sidetracking. > As David said, we need to do both. I think the SIP process for new > features and APIs may be a far better way than some hard «feature freeze». > > Jan > > > 4. des. 2019 kl. 22:49 skrev Noble Paul : > > > >> I don't think this has anything to do with code review: it has to do > with people just piling in features, but not taking the time to do any > janitorial work or remove old features that shouldn't be there anymore (I > AM LOOKING AT YOU HDFS) > > > > 100 %. If there is one problem with Solr today, it is feature bloat. > > We need to trim down Solr by atleast 50% > > > > What we need to do urgently is to create a white list of essential > > features and eliminate others. We are just getting crushed by the > > amount of code we have in Solr. We don't have so many people who can > > actively maintain such a large codebase > > > > On Thu, Dec 5, 2019 at 7:34 AM David Smiley > wrote: > >> > >> Mike D., > >> I loved your response, especially for researching what other projects > do! > >> > >> ... more responses within... > >> > >> On Tue, Dec 3, 2019 at 2:42 PM Mike Drob wrote: > >>> > >>> I'm coming late into this thread because a lot of the discussion > happened while I was offline, so some of the focus has shifted, but I'll > try to address a few topics that were brought up earlier anyway. In an > effort to be brief, I'll try to summarize sentiments rather than addressing > specific quotes - if I mischaracterize something please let me know! > >>> > >>> First, I don't believe that RTC requires consensus voting with 3 > reviews per commit or anything nearly so burdensome. A brief survey of > other Apache projects shows that most on RTC only need a single review, and > also can include exceptions for trivial changes like we are discussing > here. If we're trying to find a compromise position, I personally would > prefer to be more on the RTC side because I think it's a more responsible > place to be for a project that backs billions of dollars of revenue across > hundreds of companies annually. > >>> > >>> Spark is pretty strict RTC, but with such a volume of contributions it > may be the only option sustainable for them. > https://spark.apache.org/contributing.html > >>> HBase requires a review and has an exception for trivial patches - > http://hbase.apache.org/book.html#_review > >>> Yetus requires reviews, but allows binding review from non-committers, > and has a no review expiration. https://s.apache.org/mi0r8 - there's a > lot of other good discussion there too. > >>> Zookeeper requires a minimum one day waiting period on all code > change, but does not require explicit reviews. - > https://zookeeper.apache.org/bylaws.html > >>> > >>> One piece I'll echo from the Yetus discussion is that if we have a > habit of review, then we're less likely to get stagnant patches, and we're > also more likely to engage with non-committer contributions. If we don't > have that habit of reviews, then patches do get stale, and > trust/self-enforcement becomes the only sustainable way forward. > >>> > >>> A second point that I'm also concerned about is the idea that we don't > want JIRA issues or CHANGES entries for trivial or even minor patches. This > has a huge impact on potential contributors and their accessibility to the > project. If contributors aren't credited for all of their work, then it > makes it harder for them to reach invitation as a committer. As a personal > example, a lot of my changes were around logging and error handling, which > are minor on your list but fairly important for a supporter in aggregate. > If the community signalled that the work was less valuable, less visible, > and less likely to be accepted (each of which can follow from the previous > I think) then I don't know that I would have been motivated to try and > contribute those issues. > >> > >> > >> Our CHANGES.txt tries to simultaneously be a useful document in > informing users (and us) of what was changed for what issue that we might > actually care about, and *also* give kudos to contributors. There is a lot > of noise in there, as it is. If hypothetically a contributor files a JIRA > issue with minor/trivial priority, then maybe a git author tag is enough? > Or if not then maybe adding a special section in the CHANGES.txt for a > special thanks to contributors of unspecified issues? > >> > >>> > >>> To the point about security issues, that's something that should > probably be addressed explicitly on the security/private list, and it > absolutely needs review if only so that other developers can learn and > avoid those issues again. That's where the power of community is really
Re: Commit / Code Review Policy
Let’s keep this thread about code review guidelines, not about feature removal, please. And be concrete with proposals for re-wording Davids proposal instead of sidetracking. As David said, we need to do both. I think the SIP process for new features and APIs may be a far better way than some hard «feature freeze». Jan > 4. des. 2019 kl. 22:49 skrev Noble Paul : > >> I don't think this has anything to do with code review: it has to do with >> people just piling in features, but not taking the time to do any janitorial >> work or remove old features that shouldn't be there anymore (I AM LOOKING AT >> YOU HDFS) > > 100 %. If there is one problem with Solr today, it is feature bloat. > We need to trim down Solr by atleast 50% > > What we need to do urgently is to create a white list of essential > features and eliminate others. We are just getting crushed by the > amount of code we have in Solr. We don't have so many people who can > actively maintain such a large codebase > > On Thu, Dec 5, 2019 at 7:34 AM David Smiley wrote: >> >> Mike D., >> I loved your response, especially for researching what other projects do! >> >> ... more responses within... >> >> On Tue, Dec 3, 2019 at 2:42 PM Mike Drob wrote: >>> >>> I'm coming late into this thread because a lot of the discussion happened >>> while I was offline, so some of the focus has shifted, but I'll try to >>> address a few topics that were brought up earlier anyway. In an effort to >>> be brief, I'll try to summarize sentiments rather than addressing specific >>> quotes - if I mischaracterize something please let me know! >>> >>> First, I don't believe that RTC requires consensus voting with 3 reviews >>> per commit or anything nearly so burdensome. A brief survey of other Apache >>> projects shows that most on RTC only need a single review, and also can >>> include exceptions for trivial changes like we are discussing here. If >>> we're trying to find a compromise position, I personally would prefer to be >>> more on the RTC side because I think it's a more responsible place to be >>> for a project that backs billions of dollars of revenue across hundreds of >>> companies annually. >>> >>> Spark is pretty strict RTC, but with such a volume of contributions it may >>> be the only option sustainable for them. >>> https://spark.apache.org/contributing.html >>> HBase requires a review and has an exception for trivial patches - >>> http://hbase.apache.org/book.html#_review >>> Yetus requires reviews, but allows binding review from non-committers, and >>> has a no review expiration. https://s.apache.org/mi0r8 - there's a lot of >>> other good discussion there too. >>> Zookeeper requires a minimum one day waiting period on all code change, but >>> does not require explicit reviews. - >>> https://zookeeper.apache.org/bylaws.html >>> >>> One piece I'll echo from the Yetus discussion is that if we have a habit of >>> review, then we're less likely to get stagnant patches, and we're also more >>> likely to engage with non-committer contributions. If we don't have that >>> habit of reviews, then patches do get stale, and trust/self-enforcement >>> becomes the only sustainable way forward. >>> >>> A second point that I'm also concerned about is the idea that we don't want >>> JIRA issues or CHANGES entries for trivial or even minor patches. This has >>> a huge impact on potential contributors and their accessibility to the >>> project. If contributors aren't credited for all of their work, then it >>> makes it harder for them to reach invitation as a committer. As a personal >>> example, a lot of my changes were around logging and error handling, which >>> are minor on your list but fairly important for a supporter in aggregate. >>> If the community signalled that the work was less valuable, less visible, >>> and less likely to be accepted (each of which can follow from the previous >>> I think) then I don't know that I would have been motivated to try and >>> contribute those issues. >> >> >> Our CHANGES.txt tries to simultaneously be a useful document in informing >> users (and us) of what was changed for what issue that we might actually >> care about, and *also* give kudos to contributors. There is a lot of noise >> in there, as it is. If hypothetically a contributor files a JIRA issue with >> minor/trivial priority, then maybe a git author tag is enough? Or if not >> then maybe adding a special section in the CHANGES.txt for a special thanks >> to contributors of unspecified issues? >> >>> >>> To the point about security issues, that's something that should probably >>> be addressed explicitly on the security/private list, and it absolutely >>> needs review if only so that other developers can learn and avoid those >>> issues again. That's where the power of community is really important, and >>> I don't expect issues like that to sit around with a patch waiting for very >>> long anyway.