| Message ID | 6c1c82862814f40a408231cb249fb4b653276b52.1695379323.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add a GitHub workflow to submit builds to Coverity Scan | expand |
On Fri, Sep 22, 2023 at 10:42:00AM +0000, Johannes Schindelin via GitGitGadget wrote: > +# By default, the builds are submitted to the Coverity project `git`. To override this, > +# set the repository variable `COVERITY_PROJECT`. This may not matter all that much, because I don't expect most people to set this up for their forks (and if we have git/git results that I have access to, I will probably even stop building my peff/git one). But I just wondered if a better default would be the GitHub project name (i.e., $user/git). It has been long enough that I do not remember all of the setup on the Coverity side, but I assumed there was some "set it up for this GitHub project" button. But maybe I just picked that name myself. -Peff
Hi Peff, On Sat, 23 Sep 2023, Jeff King wrote: > On Fri, Sep 22, 2023 at 10:42:00AM +0000, Johannes Schindelin via GitGitGadget wrote: > > > +# By default, the builds are submitted to the Coverity project `git`. To override this, > > +# set the repository variable `COVERITY_PROJECT`. > > This may not matter all that much, because I don't expect most people to > set this up for their forks Except, of course, Git for Windows. And that has been the entire motivation for me to work on this here patch series in the first place, so it would be rather pointless if I could not override this in the git-for-windows/git fork. Of course, I could address this differently. I could add a commit on top and rebase that all the time. I'd just as well avoid that though. There is already too much stuff in the Git for Windows fork that I have to rebase so often. Based on your response, I was on my way to enhance the commit message accordingly, but then I saw this already being there: The Git for Windows project would like to use this workflow, too, though, and needs the builds to be submitted to the `git-for-windows` Coverity project. Would you have any suggestion how that could make the motivation and intention of this patch clearer? Ciao, Johannes
On Mon, Sep 25, 2023 at 01:52:47PM +0200, Johannes Schindelin wrote: > Hi Peff, > > On Sat, 23 Sep 2023, Jeff King wrote: > > > On Fri, Sep 22, 2023 at 10:42:00AM +0000, Johannes Schindelin via GitGitGadget wrote: > > > > > +# By default, the builds are submitted to the Coverity project `git`. To override this, > > > +# set the repository variable `COVERITY_PROJECT`. > > > > This may not matter all that much, because I don't expect most people to > > set this up for their forks > > Except, of course, Git for Windows. And that has been the entire > motivation for me to work on this here patch series in the first place, so > it would be rather pointless if I could not override this in the > git-for-windows/git fork. I didn't at all mean that it should not be possible to override it. It was obvious to me you would want to do so for git-for-windows. I meant that the default should be $user/$fork from the Actions environment, rather than just "git". That would be a more appropriate default for people who wanted to run this out of their forks (but again, the part you quoted above is basically "I'm not sure anybody even wants to do that"). > Of course, I could address this differently. I could add a commit on top > and rebase that all the time. I'd just as well avoid that though. There is > already too much stuff in the Git for Windows fork that I have to rebase > so often. > > Based on your response, I was on my way to enhance the commit message > accordingly, but then I saw this already being there: > > The Git for Windows project would like to use this workflow, too, > though, and needs the builds to be submitted to the > `git-for-windows` Coverity project. > > Would you have any suggestion how that could make the motivation and > intention of this patch clearer? I understood your intention. I think you misunderstood mine. :) The commit message is fine as-is, I think. -Peff
Hi Jeff, On Mon, 25 Sep 2023, Jeff King wrote: > On Mon, Sep 25, 2023 at 01:52:47PM +0200, Johannes Schindelin wrote: > > > On Sat, 23 Sep 2023, Jeff King wrote: > > > > > On Fri, Sep 22, 2023 at 10:42:00AM +0000, Johannes Schindelin via GitGitGadget wrote: > > > > > > > +# By default, the builds are submitted to the Coverity project > > > > +# `git`. To override this, set the repository variable > > > > +# `COVERITY_PROJECT`. > > > > > > This may not matter all that much, because I don't expect most > > > people to set this up for their forks > > > > Except, of course, Git for Windows. And that has been the entire > > motivation for me to work on this here patch series in the first > > place, so it would be rather pointless if I could not override this in > > the git-for-windows/git fork. > > I didn't at all mean that it should not be possible to override it. Aha! The "This" in "This may not matter all that much" was referring to the `git` instead of the `COVERITY_PROJECT` part. That had not been clear to me. > I meant that the default should be $user/$fork from the Actions > environment, I'd much rather default to the value needed by the Git project than a value that may be needed by any other user. I do aim to support the Git project with this patch series, first and foremost. Ciao, Johannes
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> I meant that the default should be $user/$fork from the Actions >> environment, > > I'd much rather default to the value needed by the Git project than a > value that may be needed by any other user. I do aim to support the Git > project with this patch series, first and foremost. I appreciate the sentiment. At the same time, it would be one less thing they need to tweak before starting to use it, and if there are two or more users to do so, it would already have paid off. Developers typically outnumber projects they work on. I also have to wonder if it might make it more obvious what is going on if you made the default to $user/$fork and have the project override it, which hopefully may make it easier to find out what they need to do for those who want to override it to a different value to suit their need? Thanks.
On Tue, Sep 26, 2023 at 07:19:46AM -0700, Junio C Hamano wrote: > At the same time, it would be one less thing they need to tweak > before starting to use it, and if there are two or more users to do > so, it would already have paid off. Developers typically outnumber > projects they work on. > > I also have to wonder if it might make it more obvious what is going > on if you made the default to $user/$fork and have the project > override it, which hopefully may make it easier to find out what > they need to do for those who want to override it to a different > value to suit their need? Yeah, that was my thinking (and what I had been proposing). But I really think it probably doesn't matter that much either way. I would not be surprised if there are zero developers who use this, because of the setup on the coverity side, and the fact that the results are not always immediately actionable. Even I, who has been running coverity on my local fork for a few years, will probably just switch to using the git.git run and occasionally looking at the results (that creates an extra headache because somebody has to grant acess to the git.git run results to interested parties, but it's also a one-time setup thing). -Peff
On Tue, Sep 26, 2023 at 04:02:27PM +0200, Johannes Schindelin wrote: > > > > This may not matter all that much, because I don't expect most > > > > people to set this up for their forks > > > > > > Except, of course, Git for Windows. And that has been the entire > > > motivation for me to work on this here patch series in the first > > > place, so it would be rather pointless if I could not override this in > > > the git-for-windows/git fork. > > > > I didn't at all mean that it should not be possible to override it. > > Aha! The "This" in "This may not matter all that much" was referring to > the `git` instead of the `COVERITY_PROJECT` part. That had not been clear > to me. I think we are on the same page now, but I wanted to elaborate here because this miscommunication interested me. The "this" in what I wrote was actually the "X" in: This may not matter because of $FOO, but X. The X got snipped in what you quoted, but was "But I just wondered if a better default would would be...". I certainly did not help with readability by inserting a huge parenthetical phrase and putting in a full-stop and starting the new sentence on "But" (my ninth grade English teacher would be horrified). I mention it mostly as a note to myself about trying to make sure I keep my writing clear and readable (and a cautionary tale for others!). -Peff
Jeff King <peff@peff.net> writes: > On Tue, Sep 26, 2023 at 07:19:46AM -0700, Junio C Hamano wrote: > >> At the same time, it would be one less thing they need to tweak >> before starting to use it, and if there are two or more users to do >> so, it would already have paid off. Developers typically outnumber >> projects they work on. >> >> I also have to wonder if it might make it more obvious what is going >> on if you made the default to $user/$fork and have the project >> override it, which hopefully may make it easier to find out what >> they need to do for those who want to override it to a different >> value to suit their need? > > Yeah, that was my thinking (and what I had been proposing). > > But I really think it probably doesn't matter that much either way. I > would not be surprised if there are zero developers who use this, > because of the setup on the coverity side, and the fact that the results > are not always immediately actionable. > > Even I, who has been running coverity on my local fork for a few years, > will probably just switch to using the git.git run and occasionally > looking at the results (that creates an extra headache because somebody > has to grant acess to the git.git run results to interested parties, but > it's also a one-time setup thing). Sure. I do not care too much either way, and in a situation like this where the design decision does not have a crucial longer-term impact either way exactly because it is a one-time thing for any user, whoever has invested their work on should have the final say. Thanks.
diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml index e8d0be52702..8aac00bb20f 100644 --- a/.github/workflows/coverity.yml +++ b/.github/workflows/coverity.yml @@ -11,6 +11,9 @@ name: Coverity # `COVERITY_SCAN_EMAIL` and `COVERITY_SCAN_TOKEN`. The former specifies the # email to which the Coverity reports should be sent and the latter can be # obtained from the Project Settings tab of the Coverity project). +# +# By default, the builds are submitted to the Coverity project `git`. To override this, +# set the repository variable `COVERITY_PROJECT`. on: push: @@ -20,7 +23,7 @@ jobs: if: contains(fromJSON(vars.ENABLE_COVERITY_SCAN_FOR_BRANCHES || '[""]'), github.ref_name) runs-on: ubuntu-latest env: - COVERITY_PROJECT: git + COVERITY_PROJECT: ${{ vars.COVERITY_PROJECT || 'git' }} COVERITY_LANGUAGE: cxx COVERITY_PLATFORM: linux64 steps: