Re: Re-evaluating the practice of automating user configuration
On 2023-10-21 17:32:13 +0200, Ricardo Wurmus wrote: > > Maxim Cournoyer writes: > > > All in all, I guess my position is unchanged: despite the potential for > > surprises, automating and enforcing these configs provide benefits that > > outweigh the cons, in my experience/opinion. > > I concur. > > In light of efforts to reduce cognitive overhead, I think it is a good > idea to automatically use the default configuration. Contributors can > opt out, How can I do that? > but I prefer not to have to think about yet another important > piece of configuration here. > > -- > Ricardo > -- There are only two hard things in Computer Science: cache invalidation, naming things and off-by-one errors. signature.asc Description: PGP signature
Re: [PATCH qa-frontpage] Apply incoming patches onto the correct feature branch
Christopher Baines writes: > Vivien Kraus writes: > >> It seems that the only available information of the feature branch is >> in the patch name. >> --- >> >> Hello guix! >> >> Thank you for making the QA tool. It seems to me that feature branches >> are ignored for patches. The patches seem to always be applied on top >> of master. I get that idea because the base-for-issue-* tag is always >> put on HEAD, and all my patches targetting gnome-team recently get >> applied to master. I understand that the latter could be a problem >> with me. >> >> If patches are indeed applied on top of master, then the question >> arises, what to do. The available information from patchwork is >> scarce: the base-commit is not available, and the optional feature >> branch only appears in the name of the patches of the series. >> >> I think it is possible to parse the patch names. This way, we get some >> useful information: the feature branch, the series revision, and the >> patch index. The patch index can be used to make sure the patches are >> in correct order (this can be the case if a server in the path >> re-orders the emails). >> >> What do you think? > > Hi Vivien, > > Thanks for trying to implement this and sending a patch, I think it's a > good feature to add but there's a few things needed to make this work. > > Firstly, you need to actually change which branch the patch is applied > to, and the place to do that is here [1] in the call to > with-git-worktree. You can probably use get-commit from > (guix-qa-frontpage git-repository) to find the commit hash for a > branch. You'll also probably need to move around some of the code in > create-branch-for-issue so that you have the data about the patches to > work out what the desired branch is before you call with-git-worktree. > > 1: > https://git.cbaines.net/guix/qa-frontpage/tree/guix-qa-frontpage/manage-patch-branches.scm#n250 > > The second thing is that there's other places in the codebase that > assume the patches have been applied to the master branch. In > start-manage-patch-branches-thread, there's > get-changes-compared-to-master which is used to decide if a branch needs > recreating if there's too many changes between the base revision and a > recent master branch revision. That'll need changing to at least skip > over any patches been applied to a non-master branch, or perform the > comparison against the tip of the relevant branch. > > The third thing that will need to change to allow this to happen is > adding clear messaging on to the issue page to indicate where patches > have been applied to non-master branches. That'll help to avoid people > merging these patches in to the master branch rather than the branch > they were intended for. > > Maybe what would be helpful is a procedure in (guix-qa-frontpage issue) > that takes the data for the issue from latest-patchwork-series-by-issue, > and gives you back the branch name that the patches should be applied > to. That can then be used to get the branch information for all the > above 3 use cases. > > As for trying to order the patches, do you know of a case where the > ordering from Patchwork is wrong? I think it's only worth changing the > ordering in the qa-frontpage if we know we're doing it for a reason. > > There's now a create-issue-branch command to the guix-qa-frontpage > script as well, which will be a good way of testing the code out for > applying patches to various branches. It'll fail when it comes to > pushing the branch, but it'll still be useful for testing the code up to > that point. > > Would you be able to look at sending some revised patches? I've now gone ahead and applied a slimmed down version of this patch, and made the other changes to get a basic form of applying patches to specific branches in place. There's probably going to be some problems and more error handling to add, but this should be a good starting point. Thanks, Chris signature.asc Description: PGP signature
Re: Re-evaluating the practice of automating user configuration
Maxim Cournoyer writes: > All in all, I guess my position is unchanged: despite the potential for > surprises, automating and enforcing these configs provide benefits that > outweigh the cons, in my experience/opinion. I concur. In light of efforts to reduce cognitive overhead, I think it is a good idea to automatically use the default configuration. Contributors can opt out, but I prefer not to have to think about yet another important piece of configuration here. -- Ricardo
Re: Re-evaluating the practice of automating user configuration
Hello, Tomas Volf writes: [...] > I have to admit I was surprised, and not in a pleasant way. When I started > playing with Guix, I went over the etc/git/gitconfig and copied the parts I > liked into the .git/config. For some reason I do not like tools automatically > touching the .git directory, and now I have the include there (5 times). I think it's mostly surprising because Git itself doesn't provide for a way to provision sane per-project settings, which is what the build system of Guix does. -- Thanks, Maxim
Re: Re-evaluating the practice of automating user configuration
Hi Liliana, Liliana Marie Prikler writes: > Hi Guix, > > as we all are more or less aware of, Guix automates quite much of the > user's configuration for comfortably hacking on our codebase. As has > been argued elsewhere, both by myself and fellow Guix, this is not > always a good thing. > > Let's start with the cleanest example of how to do things the right > way: Our Emacs configuration is split across two files (one of which is > a directory, but let's get back to that). One of them are the > directory-local variables stored in .dir-locals.el, the other the > snippets in etc/snippets–if you're using YASnippet, the former loads > the latter. I have no qualms with this being automated, as Emacs > itself gives me plenty opportunity of opting out of it. I could > declare any of the included variables or forms unsafe and ignore them > in future sessions. Likewise, I can mark them as safe to affirm my > consent that these variables be changed in /path/to/guix/checkout. > > None of this holds for the git config, which we install unasked in the > working tree with a DATA target that we want neither distributed nor > installed otherwise. This has led to confusion both in the mailing > lists and the IRC on multiple occasions, so I'd propose we instead use > PHONY targets for: > 1. git-hooks to install the git hooks that committers need. > 2. git-config to install all of the git config > a. git-config-diff to just install the diff xfuncs > b. git-config-format to just install the format block > c. git-config-pull to just install the pull block > d. git-config-sendemail to just install the sendemail block > 3. git-fullconfig for both 1 and 2. As argued before, going this route would have the following downsides: 1. the pre-push-hook would no longer be installed out of the box, which could mean forgotting to sign a commit and having to ask Savannah folks to drop the offending commit(s). That's a blocker for me, at least until we have a server-side hook that can guard against this. 2. The pre-push-hook could go stale (not self-updating). That's likely to happen as people would seldom run 'make git-hooks' to refresh them. 3. We'd loose some notifications for teams, likely for first submissions from users that have yet to run 'make git-hooks', or from users who chose not too. 4. We'd have more problems applying patches since the 'useAutoBase = true' is not enabled by default, and documentation is a weak assurance that users will do this. > Internally, these would still be based on the actual file names to get > time-stamps to work. Thus, on a fresh pull or if you haven't pulled in > a while, you can run either `git fullconfig` or any of the above to set > things up. > > Incidentally, my .git/config currently reads the following: > > [include] > path = ../etc/git/gitconfig > path = ../etc/git/gitconfig > path = ../etc/git/gitconfig > path = ../etc/git/gitconfig That should be fixed in Git. 'git config --add include.path ../etc/git/gitconfig' should not be re-adding the same entries over and over if they are already there. All in all, I guess my position is unchanged: despite the potential for surprises, automating and enforcing these configs provide benefits that outweigh the cons, in my experience/opinion. -- Thanks, Maxim
