| *** xjuan_ has quit IRC | 00:19 | |
| WSalmon | nanonyme, if the issue is with black rather than pylint you can just ask black to "fix" you code | 07:23 |
|---|---|---|
| WSalmon | `tox -e format-check` will just check your code but `tox -e format` will "fix" it | 07:25 |
| *** benschubert has joined #buildstream | 07:29 | |
| *** santi has joined #buildstream | 08:38 | |
| douglaswinship | tristan, WSalmon: thanks for the advice. I tried both tox and black, and got the same results. | 10:25 |
| douglaswinship | Oddly, I didn't get the same results when I tried using Black from inside vim. That's what threw me off when I first looked at this. | 10:26 |
| douglaswinship | Calling Black from inside vim seems to be stricter, it was enforcing a shorter line-length limit, and made more changes. | 10:27 |
| WSalmon | so by using tox you make sure you have the "right" consistent version of black and use the right config | 10:27 |
| WSalmon | https://gitlab.com/BuildStream/buildstream/-/blob/master/tox.ini#L124 | 10:28 |
| douglaswinship | I thought there might be some config involved, yeah. | 10:30 |
| WSalmon | i thought we had a config, eg line length etc but i cant see it in the tox | 10:30 |
| WSalmon | black is supposed to have minimal config tho | 10:30 |
| coldtom | https://gitlab.com/BuildStream/buildstream/-/blob/master/pyproject.toml WSalmon | 10:32 |
| coldtom | because python has ~30 standard ways to configure tools :P | 10:33 |
| WSalmon | oh boi | 10:33 |
| WSalmon | thanks coldtom | 10:33 |
| douglaswinship | The pipeline just passed on https://gitlab.com/BuildStream/buildstream/-/merge_requests/1989 | 11:47 |
| douglaswinship | So I think it's ready to merge! | 11:48 |
| WSalmon | jjardon, FYI https://gitlab.com/BuildStream/buildstream/-/merge_requests/1987#note_379062787 | 12:00 |
| nanonyme | WSalmon, I don't like external tools to "fix" my code. I want to know what the rules are so I can write correct code. It's fine to have automation for normalizing large codebases | 12:03 |
| nanonyme | Especially when writing a pull request which constitutes of multiple commits, it's quite suboptimal to retroactively fix code to abide to style rules | 12:04 |
| WSalmon | i mean https://black.readthedocs.io/en/stable/the_black_code_style.html it dose tell you the rule if you really want to do it your self. but i like it for diffs as once you get used to running it the diffs generally get smaller and less about style | 12:07 |
| douglaswinship | question about merge-requests: Should I be setting my MRs to require approval? | 12:14 |
| douglaswinship | I learnt to use gitlab MRs when i was working on Freedesktop-sdk, and i never had to edit the 'required approvers' number, it defaulted to 1. I didn't even notice it was a setting that you can change. | 12:15 |
| douglaswinship | But i've noticed that on buildstream, it defaults to zero. | 12:15 |
| douglaswinship | In other news, I've made a separate MR for removing the Aarch64 overnight test, as requested by jjardon. https://gitlab.com/BuildStream/buildstream/-/merge_requests/1994 | 12:17 |
| douglaswinship | WSalmon: that was your commit. Any chance you could add some detail into the description? | 12:17 |
| WSalmon | douglaswinship, looks like a good issue | 12:20 |
| WSalmon | sorry MR | 12:20 |
| WSalmon | a little fyi, but dont worry as many people dont do this but if you reply to the comment rather than making a new one then i can just mark the hole lot as resolved, https://gitlab.com/BuildStream/buildstream/-/merge_requests/1987#note_379066729 | 12:25 |
| douglaswinship | WSalmon: THANKYOU! | 12:27 |
| douglaswinship | For the longest time, I thought some comments couldn't be replied to, and some could. | 12:27 |
| WSalmon | douglaswinship, that used to be true | 12:27 |
| douglaswinship | When did it change? | 12:27 |
| douglaswinship | Would be nice if they'd actually put the word "reply" somewhere in the comment, instead of relying on an icon. | 12:28 |
| WSalmon | duno, you used to have to choose about ether a comment or `start a dissuction` | 12:28 |
| douglaswinship | Glad I've finally noticed it now. | 12:28 |
| WSalmon | but now you can reply to comments as well as dissuctions | 12:28 |
| WSalmon | if you do `start a thread` then the reply box is already there, but for comments you have to use the tiny reply button | 12:29 |
| douglaswinship | ugh... | 12:29 |
| douglaswinship | gitlab is great. But still got some real issues. | 12:29 |
| WSalmon | +1 | 12:29 |
| douglaswinship | How would I restart marge-bot on https://gitlab.com/BuildStream/buildstream/-/merge_requests/1989 btw? | 12:48 |
| douglaswinship | Do I just reassign it to her? | 12:48 |
| coldtom | yep douglaswinship | 12:49 |
| douglaswinship | Does marge-bot automatically rebase things onto master before merging? Or will i have to do that manually? | 13:06 |
| douglaswinship | I've not really paid attention to marge-bot before. | 13:06 |
| coldtom | marge rebases before merge | 13:06 |
| douglaswinship | good good :) | 13:06 |
| jjardon | douglaswinship: I think we want to fix the aarch64 job, not remove the job | 13:47 |
| douglaswinship | jjardon: as you said, it's an independant piece of work, from !1987 | 14:00 |
| douglaswinship | !1987 is the main thing I'm trying to get through | 14:00 |
| douglaswinship | I separated out WSalmon's commit, and gave it its own MR, because I didn't want to just cut it out of my MR and remove it entirely. | 14:01 |
| douglaswinship | That seemed inconsiderate to WSalmon's work. | 14:01 |
| jjardon | nice | 14:02 |
| * jjardon looking forward to merge that MR :) | 14:02 | |
| douglaswinship | I'll leave WSalmon / anyone who's invested in fixing that job, to decide what to do with !1994 | 14:10 |
| douglaswinship | (sorry, doorbell rang in the middle of me typing) | 14:10 |
| *** santi has quit IRC | 16:32 | |
| *** santi has joined #buildstream | 16:34 | |
| *** santi has quit IRC | 17:58 | |
| *** benschubert has quit IRC | 19:38 | |
| *** adds684 has joined #buildstream | 22:57 | |
| *** adds684 has quit IRC | 23:03 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!