*** reuben640[m] has joined #buildstream | 00:06 | |
*** theawless[m] has joined #buildstream | 00:40 | |
*** traveltissues has joined #buildstream | 08:38 | |
*** pointswaves has joined #buildstream | 08:45 | |
*** phildawson-ct has joined #buildstream | 09:00 | |
*** phildawson has quit IRC | 09:01 | |
*** pointswaves has quit IRC | 09:08 | |
*** pointswaves has joined #buildstream | 09:09 | |
*** pointswaves has quit IRC | 09:24 | |
*** tme5 has joined #buildstream | 09:44 | |
*** jonathanmaw has joined #buildstream | 10:10 | |
*** swick has quit IRC | 11:18 | |
*** swick has joined #buildstream | 11:19 | |
tme5 | benschubert, following our discussion yesterday I just pushed to !1808 | 11:22 |
---|---|---|
gitlab-br-bot | MR !1808: Improvements to _GitSourceBase and _GitMirror https://gitlab.com/BuildStream/buildstream/merge_requests/1808 | 11:22 |
*** swick has quit IRC | 11:23 | |
*** santi has joined #buildstream | 11:24 | |
benschubert | tme5: any specific reason why you have changed some 'ref' to 'rev' ? | 11:35 |
tme5 | to be more in line with Git terminology. A ref is a named label like a branch or a tag. A rev or revision is a more general term, referring to commits, and I think trees, tag objects themselves, etc. | 11:37 |
tme5 | so the functions which can take git revisions, I made the variable names reflect this | 11:38 |
benschubert | Thanks, makes sense :) | 11:39 |
benschubert | tme5: this 'fetch_all' condition, do you think we could either split it in smaller methods to make it more understandable, or if we could make it more readable? I'd be worried at having to change something there in the future | 11:43 |
tme5 | i'll give it a shot | 11:47 |
benschubert | Cheers, that's my last remark on the PR, seems good otherwise, thanks for fixing the perf regression there | 11:48 |
*** swick has joined #buildstream | 11:55 | |
*** phildawson-ct has quit IRC | 12:00 | |
*** phildawson-ct has joined #buildstream | 12:00 | |
tme5 | the logic isn't so much worse than the no-ref-in-track check in validate_cache | 12:14 |
benschubert | I do agree with you, and am not happy with the logic in the other one either :'D | 12:14 |
benschubert | And to be clear: this is not a blocker on the PR, it was just me hoping we could improve things even more :) | 12:15 |
tme5 | I split out the checking of whether the remote has the tag into another function | 12:15 |
tme5 | the logic is complicated by the fact the result of trying to shallow-fetch affects whether to full-fetch, so it can never be an if-else situation | 12:16 |
tme5 | ah ok, well hopefully this makes things nice | 12:17 |
tme5 | r | 12:17 |
benschubert | Also, whenever I put a "nit:" comment on the MRs, feel free to resolve without tackling, it's usually tips or other options, that are not required but might give another view | 12:18 |
benschubert | > the logic is complicated by the fact the result of trying to shallow-fetch affects whether to full-fetch, so it can never be an if-else situation | 12:19 |
benschubert | Yep, I understand, and the shallow-fetch is definitely a nice thing | 12:19 |
benschubert | https://stackoverflow.com/questions/31278902/how-to-shallow-clone-a-specific-commit-with-depth-1 is also an intersting thing for the future :'D | 12:20 |
*** swick has quit IRC | 12:33 | |
*** phildawson-ct has quit IRC | 12:37 | |
*** delli3 has joined #buildstream | 12:44 | |
*** delli3 has quit IRC | 12:45 | |
*** delli3 has joined #buildstream | 12:45 | |
tme5 | finished things off in !1808, i think it's ready? | 13:02 |
gitlab-br-bot | MR !1808: Improvements to _GitSourceBase and _GitMirror https://gitlab.com/BuildStream/buildstream/merge_requests/1808 | 13:02 |
*** santi has quit IRC | 13:20 | |
*** santi has joined #buildstream | 13:23 | |
*** phildawson-ct has joined #buildstream | 13:50 | |
tme5 | looks like Git on CentOS 7.6 doesn't support cloning a shallow repo, and so can't stage anything | 14:11 |
tpollard | We've hit this problem before | 14:14 |
tpollard | & then introduced that ci target | 14:14 |
tpollard | https://gitlab.com/BuildStream/buildstream/issues/833 | 14:15 |
tme5 | you think I should skip if HAVE_OLD_GIT ? | 14:21 |
benschubert | tme5: the problem if you do that is, that we won't be able to open workspaces on CentOS if this change goes in. I think I'd rather disable the shallow fetch if git is too ancient | 15:12 |
benschubert | which indeed adds one more condition in that list x') | 15:13 |
tme5 | right, ok | 15:18 |
tme5 | how should i check git version in the source? ccan i use buildstream.testing? | 15:19 |
benschubert | No, testing stuff should not be used for normal executions | 15:20 |
*** phildawson has joined #buildstream | 15:41 | |
*** phildawson-ct has quit IRC | 15:42 | |
*** santi has quit IRC | 15:51 | |
*** santi has joined #buildstream | 15:51 | |
*** santi has quit IRC | 16:19 | |
*** santi has joined #buildstream | 16:22 | |
*** phildawson has quit IRC | 16:51 | |
*** phildawson has joined #buildstream | 16:55 | |
*** traveltissues has quit IRC | 16:56 | |
*** phildawson has quit IRC | 17:08 | |
*** santi has quit IRC | 17:24 | |
*** pointswaves has joined #buildstream | 17:27 | |
*** santi has joined #buildstream | 17:37 | |
*** tme5 has quit IRC | 17:37 | |
*** pointswaves has quit IRC | 17:41 | |
*** jonathanmaw has quit IRC | 18:43 | |
*** santi has quit IRC | 19:25 | |
*** santi has joined #buildstream | 19:25 | |
*** phoenix has joined #buildstream | 19:27 | |
*** phoenix has quit IRC | 19:31 | |
*** pointswaves has joined #buildstream | 20:16 | |
*** dylan-m has joined #buildstream | 21:18 | |
*** pointswaves has quit IRC | 21:48 | |
*** pointswaves has joined #buildstream | 21:48 | |
*** pointswaves has quit IRC | 21:50 | |
*** pointswaves has joined #buildstream | 21:50 | |
*** pointswaves has quit IRC | 21:57 | |
*** santi has quit IRC | 23:38 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!