<qyliss>
just testing it builds now and I'll push it
<hexa->
thanks
pbb has joined #nixos-dev
<andi->
infinisil: minus the fact that these results are not worth as much as the runtime environment might as well have sandboxing disabled and/or weird settings for /bin/sh
<qyliss>
or binfmt, or ...
<andi->
the possibilities are endless
<andi->
I have the impression that the move to github checks has removed visbility from the ofborg checks
<qyliss>
hexa-: that should do it
<hexa->
oh boy. thanks for noticing that qyliss++
<{^_^}>
qyliss's karma got increased to 121, that's Numberwang!
<gchristensen>
again I think a label would help as a visual indication something went wrong without compromising the sanctity of the X
<supersandro2000>
you don't get any notification for mails or comments
<supersandro2000>
someone that is not checking there PRs daily would miss that
<adisbladis>
supersandro2000: I get so many github notifications already that they are useless
<adisbladis>
It's a sea of spam
<qyliss>
suggestions as comments would make sense to me, as long as false positives are there
<andi->
adisbladis++
<{^_^}>
adisbladis's karma got increased to 135, it's a crit!
<supersandro2000>
adisbladis: I get probably more than you and I am totally fine with it
<andi->
I actually filter out "nixpkgs-review" in mails. Those are just binned
<supersandro2000>
if I wouldn't get any I would never take a second look at any PR
<supersandro2000>
I just trash all nixos/nixpkgs mails because they are duplicated with github notifications
<andi->
It is not about who gets the most mails it is about not wasting attention of others
<adisbladis>
supersandro2000: Don't dismiss other people like that. Just because you're fine with it doesn't mean everyone else is.
<andi->
also if you can still use GH notifications: You aren't receiving enough notifications :D
<supersandro2000>
I get about 100 an hour
<gchristensen>
as adisbladis said, it doesn't matter
<supersandro2000>
but github statuses don't work for feedback
<andi->
If the only person that can keep up with the mails is you then welll.. we are down to one person mainting things
<adisbladis>
supersandro2000: A key to being a successful foss contributor/maintainer is to listen to other peoples input, even when you disagree with it.
<adisbladis>
You can't just double down
<supersandro2000>
everything that github allows is not what we want
<supersandro2000>
I want a notification, you dont, GitHub 🤷
<eyJhb>
Putting this here, I also think you ( supersandro2000 ) have more time on your hands, to go through a ton of nonesense emails + notifications :)
<eyJhb>
And the past so far has shown, that many of the active bigger contributors really do not enjoy these notifications, as it fills up the notifications area, and might take focus away for some important PRs.
<eyJhb>
In gerenal I would guess that adisbladis, and mayny others, are dealing with nixpkgs + other projects a lot different than you
<gchristensen>
maybe ofborg could send email
<eyJhb>
Also, CI does provide valuable information, the same that can be conveyed in comments
<andi->
gchristensen++
<{^_^}>
gchristensen's karma got increased to 426
<andi->
like those hydra notifications :D
<eyJhb>
So it is just a matter of HOW these should be send/done :) - would be cool gchristensen
<qyliss>
gchristensen++
<eyJhb>
Not sure if it would be 100% helpful however, but could be a nice opt-in thing/opt-out thing
<{^_^}>
gchristensen's karma got increased to 427
<qyliss>
I'd love to get mail from OfBorg
<eyJhb>
And it doensn't really need to be emails, does it?
<gchristensen>
qyliss: ofborg would love to send you email too <3
<qyliss>
<3
<supersandro2000>
and to which inbox do you want to send the mail?
<adisbladis>
eyJhb: Download the Nix Reviewer android app and get phone notifications!
<qyliss>
supersandro2000: we have email addresses and github IDs for every maintainer
<gchristensen>
supersandro2000: I look forward to your proposal!
<supersandro2000>
qyliss: we do not
<supersandro2000>
github does not allow the flexible solution everyone wants
<qyliss>
supersandro2000: yes we do
<eyJhb>
Honestly, ofborg could just make a comment if a stage fails...
<qyliss>
in maintainers/maintainer-list.nix
<eyJhb>
It is mostly the fails we are interested in
<eyJhb>
qyliss:first time contributors :D
<andi->
The problem with some people being mailed will be that some others would rather still have the comments of some other ci script.
<gchristensen>
an "it is impossible" attitude never got anyone anywhere
<eyJhb>
andi-: I think the goal here is, to not have 3 different "bots" :)
<supersandro2000>
well mail is required but githubID is optionally and people go crazy if you request them to add it
<eyJhb>
gchristensen: NixOS is impossibly awesome :D
<qyliss>
supersandro2000: really?
<andi->
eyJhb: yeah just saying that someone might want "quicker" feedback and thus still continues to do that.
<qyliss>
doesn't it get filled in automatically somehow?
<gchristensen>
yeah, it is okay to omit github ID
<eyJhb>
Oh yeah, increase the ofborg capacity? :p
<eyJhb>
*not really ideal*
<andi->
supersandro2000: that person didn't go crazy he just didn't want to provide it as it is optional
<gchristensen>
some people don't want their maintainer entry to be so attached to them
<supersandro2000>
qyliss: I would say 3 out of 5 times kinda. Not like the information is tied to the PR anyway
<andi->
It is an opt-in feature in that case. Some features have requirements. If you want mails but we do not have an address that doesn't work.
<supersandro2000>
if they create a PR on GitHub it is attached to them anyway
<qyliss>
commits have email addresses anyway
<qyliss>
that would be another sensible place to get them from
<andi->
if PRs were mail threads we would have addresses..
<gchristensen>
lol
<supersandro2000>
if nixpkgs would be mailing list based I wouldn't be here
<andi->
Why not? A few more mails to those (what was it?) 200 an hour is feasible :)
<qyliss>
let's not derail this
<supersandro2000>
I don't think anyone could manage the amount of mails that mailing list would create
<supersandro2000>
and older contributions would be easily lost
<qyliss>
you're wrong, because people on other projects clearly do manage, but let's not derail this
<srk>
I do like that this space is a bit democratized now instead of just one bot
<supersandro2000>
also which is the third bot? It can't be me because I am not a bot.
<andi->
What is a bot?
<supersandro2000>
r-ryantm or r-rmcgibbo
<andi->
A bot has a leading r-?
<supersandro2000>
no
<srk>
oh, so actually 4 bots
<supersandro2000>
they are just running without any user interaction
<hexa->
what's the difference?
<hexa->
the other botowners also reply to feedback
<qyliss>
the problem here isn't that there are bots, it's that there is spam
<qyliss>
r-ryantm has never spammed me
<V>
^
<hexa->
^^
<srk>
what about the package update spam that it creates? :)
<hexa->
the ofborg integration is also very subtle
<hexa->
srk: there is alot of worth in that
<V>
it's spam if it's undesirable
<ryantm>
I wish it was true that it never spammed but ❤️.
<qyliss>
<3 ryantm
<{^_^}>
ryantm's karma got increased to 34
<hexa->
we've dodged lots of security issues by being ahead on package updates
<{^_^}>
ryantm/nixpkgs-update#65 (by FRidh, 2 years ago, open): Backport PRs
<srk>
sure there's a lot of worth in that but it also makes other PRs scroll away very fast. I just find it confusing when browsing PRs
<srk>
but you can't move it to another repo because package updates are often related to module updates so I'll just learn to filter
<qyliss>
srk: you can add -author:r-ryantm to your search
<qyliss>
yeah
<srk>
qyliss++ thanks :)
<{^_^}>
qyliss's karma got increased to 122
<supersandro2000>
I also heard people who dislike r-ryantm
<ryantm>
One of the OCaml maintainers for instance.
<gchristensen>
thankfully ryantm works pretty hard to address problems, respond to feedback, and fit r-ryantm in to the workflow of the community as a whole
aranea has quit [Quit: server maintenance]
aranea has joined #nixos-dev
rmcgibbo has joined #nixos-dev
<rmcgibbo>
I'm happy to turn off github comments for r-rmcgibbo builds that result in successes for PRs associated with any particular user. There's definitely a balance between making this visible and spamming, and it's different for anyone especially based on how much github email they're getting.
<rmcgibbo>
@adisbladis: i'll add you to that list.
<rmcgibbo>
If, on balance, ya'll prefer that I have r-rmcgibbo only comment on github for any user when the build fails, we can obviously make that change too. I personally think there's a lot of value in the nixpkgs-hammering based "suggestions" that don't count as build failures and other suggestions based on grepping the build logs for common mistakes, but
<rmcgibbo>
I'm happy to do whatever the community prefers.
<hexa->
yeah, nixpkgs-hammering is neat
<ryantm>
I was happy when OfBorg moved to GitHub checks, I think it probably makes sense for Robot Robert's comments to become checks too.
<supersandro2000>
Can you easily post them as checks?
<supersandro2000>
it would be neat if checks allowed a longer summary right at the bottom.
<rmcgibbo>
Yeah, I'd like to turn it into a github check but -- just to be honest -- it's not going to happen overnight. It requires some integration with ofborg since as-is, I'm just a rando and do not have the authorization to post checks. And furthermore, I'd like to make the results as accurate as possible first (e.g. handling build logs when the tool
<rmcgibbo>
times out or runs out of disk in a better way) so as to not cause false positive "X"s.
<sterni>
or we add that to ofborg of course but I'm not sure if it is a good idea to have ofborg as a bottleneck for stuff that is somewhat unrelated
<sterni>
rmcgibbo: i also wonder if we could merge the comments for two architectures by having the second run edit the first comment
<sterni>
notifications situation would be worse but also less noise which seems to be the base dilemma
<rmcgibbo>
@sterni: That's a good idea, and I can add that to my TODO list.
<sterni>
do you use nixpkgs-review's subcommand for posting the comments?
<supersandro2000>
I should probably think about doing that too
<supersandro2000>
should be doable with graphql
<rmcgibbo>
Yeah, I'm using a modified version of nixpkgs-review that has some more hooks installed so that I can run nixpkgs-hammering and stuff, but in the end the comment is posted with the Github.py code in nixpkgs-review. It's not that hard to modify though.
<gchristensen>
I don't suppose it can create inline ```suggestion blocks
<rmcgibbo>
gchristensen: I assume we could create suggestion blocks (i.e. a "review" rather than a "comment"). It might be more noisy rather than less though.
<gchristensen>
I wonder how often they're not wanted? The perk of suggestion blocks is applying the fixes are really trivial, and easy to get contributors to make the fixups
<rmcgibbo>
With respect to using the Checks API, does anyone know if there's a good way to represent the nixpkgs-hammering suggestions? It seems like in many cases they should not be taken as obligatory, so using the red X is definitely out of the question.
<gchristensen>
Of course if they're not highly reliably correct and wanted, the value goes down precipitously
<supersandro2000>
hammering could probably not be converted into suggestions except some of the trivial ones
<supersandro2000>
most of them require research and can't be auomated
<rmcgibbo>
It's probably straightforward to make nixpkgs-hammering use the Review API to highlight the line numbers where the suggestion is located rather than leaving the linenumber information in a comment, but yeah it's a lot harder to propose what patch should be made to fix the issue. That would require a lot of new rust code.
<rmcgibbo>
It's definitely _possible_ though for many of the checks (remove this token, reorder these attributes, add this string, etc).
<rmcgibbo>
If I'm understanding correctly, I don't think reviewdog makes it that much easier. We'd still need to make nixpkgs-hammering be able to produce a diff between the current state of the file and the new "fixed" state, and doing that correctly for all of the checks is the hard prt.
<supersandro2000>
I didn't mean that part. I was talking about posting the suggestions to the correct lines as review comments
rajivr has quit [Quit: Connection closed for inactivity]
<supersandro2000>
but without a good diff I don't see a big advantage.
<rmcgibbo>
Got it. I agree.
<rmcgibbo>
I think posting the "build successes" as green Github Checks for reduced visibility and the "build failures" as comments might be a good direction forward.
<rmcgibbo>
However I'm not sure where the nixpkgs-hammering suggestions best fit into that. Where should they be posted?
<sterni>
rmcgibbo: you should be able to just post the suggestion on the current line for starters right? the diff feature can be added later probably
<sterni>
rmcgibbo: I think you can also have failed tests link a log and then the “log output“ could show the suggestions
<sterni>
less accessible though
<sterni>
but probably better than failing ofborg builds where you have to go to the github checks log page and then click on the link to ofborg to see the actual log :p
<supersandro2000>
I can just expand the checks and click on it
<sterni>
supersandro2000: does it link you directly to ofborg from the PR view?
<rmcgibbo>
@sterni: Personally I feel like the suggestions are very extensive and quite pedantic (which is good, but still), so I think it's preferable to see them in the UI as a semi-hidden comment with "details" that you need to click to expand, so I'm not sure if posting the suggestions as a "review" on the particular lines is the best way? It often takes
<rmcgibbo>
up a lot of screen real estate and is very "in-your-face".
<sterni>
yeah that's fair, I think a problem is also that they are sometimes somewhat unrelated (apart from the cases where they (used to) reference untouched files): you probably don't want to refactor a nix expression in a PR for a minor version bump
<rmcgibbo>
I suppose the only check that I've currently deployed that I'm not 100% sold on is the missing-phase-hooks check.
<rmcgibbo>
For the other ones -- the most commonly triggered is probably unclear-gpl, it's probably best for the person doing the minor version bump to fix it since they're the human who has been looking at upstream and it doesn't really require a refactoring.
cole-h has joined #nixos-dev
<supersandro2000>
rmcgibbo: there are a lot of people who don't like that and I got a lot of push backs when mentioning such little clean ups
<supersandro2000>
or a few. take the number with a grain of salt
<rmcgibbo>
Yeah, there's a bit of a selection bias because obviously the people who appreciate those little clean ups (and the whole community who gets a nixpkgs with more accurate license information) don't usually reach out to you and congratulate you for all your hard work ;)
<jtojnar>
looks like the checks API does actually support annotations
<jtojnar>
should be pretty straightforward to adapt nixpkgs-review to submit that, so we only need to get oauth credentials for nixpkgs
<rmcgibbo>
Do you think those annotations would be better than a "suggestions" comment? I find them very visually in-your-face.
<rmcgibbo>
Maybe I'm looking at the wrong thing -- I should find an example of the checks API annotations look like in the web UI so I can make sure I'm talking about the right thing.
<rmcgibbo>
I may be confusing the UI for the check API annotations with the UI for a "review".
<jtojnar>
I think I saw the editorconfig check use them
<jtojnar>
but they can be hidden and at least do not spam user’s notifications
<rmcgibbo>
Well, something to investigate in a separate repository first I suppose.
rmcgibbo has quit [Quit: Connection closed]
rmcgibbo has joined #nixos-dev
rmcgibbo[m] has joined #nixos-dev
rmcgibbo has quit [Quit: Connection closed]
pmy has quit [Ping timeout: 272 seconds]
pmy has joined #nixos-dev
dmj` has quit [Ping timeout: 240 seconds]
raboof has quit [Ping timeout: 240 seconds]
thoughtpolice has quit [Read error: Connection reset by peer]
thoughtpolice has joined #nixos-dev
raboof has joined #nixos-dev
dmj` has joined #nixos-dev
<abathur>
I'm a bit late on this, but I guess in theory people can just block the bot accounts and not get notified, yeah?
<abathur>
(not sure, I've never had to block anyone on gh, but I know the option's there...)
<abathur>
related, but not exactly the same topic; I think it's a net good to minimize the friction of creating, testing, and adopting new PR/issue process refinements
<qyliss>
if you block somebody, they aren't allowed to post on your PRs
<supersandro2000>
even if you are a member of the nixos org?
<Irenes[m]>
that's correct
<Irenes[m]>
and it's the right decision - people have to block coworkers all the time
<Irenes[m]>
I mean, it's rare but it's routine
<Irenes[m]>
for example, in large workplaces it's common to share an employer with your ex
<Irenes[m]>
or with your abuser
<Irenes[m]>
from github's standpoint they want one set of rules that works regardless, and strong blocks are the clear right choice
<Irenes[m]>
that does mean that, because blocks are such a strong measure, they aren't the ideal solution for less serious situations
<supersandro2000>
I don't believe that works like this
<supersandro2000>
I can still comment under PRs of people who blocked me
<qyliss>
> After you've blocked a user, they cannot: [...] Comment on or edit issues or pull requests that you've created
<{^_^}>
error: syntax error, unexpected ',', expecting ')', at (string):472:28
<supersandro2000>
that probably only applies to repositories they are owner of
<Irenes[m]>
if there's a technicality here, my guess is that it's the organization issue, yeah
<Irenes[m]>
the "personal account" in the title of this document suggests that the whole thing may be useless in organizations
<Irenes[m]>
though github's model is such that there's no technical distinction between personal accounts and accounts that are members of one or more organizations
<Irenes[m]>
anyway
<Irenes[m]>
do I correctly understand that the issue is notifications from the bot's posts?
<Irenes[m]>
scrollback is long and hard to catch up on
<rmcgibbo[m]>
Yeah, I think some folks -- especially people who are very prolific on github -- consider the notifications from the bots a bit spammy.