Re: [arch-general] [PATCH 01/48] Bashification of initscripts
Am 17.07.2010 16:24, schrieb Victor Lowther:
Sorry for taking so long. Apart from squashing the trivial bashifications, all patches I didn't comment on are ACKed.
That is cool. I went ahead and reworked/squashed everthing I considered a "trivial bashification" into 6 or 7 patches after taking into account the feedback from the ML. The result is the bashification-redux branch @ git://fnordovax.org/~victor/arch-initscripts -- it is now 16 patches instead of 48, but the end result is exactly the same. I can post it to the ML if you like.
I looked over the new branch and found it much more readable. And although some of the patches are not 100% clean yet (at least one contains a trivial change that wasn't advertised in the commit), I like it. Just a remark about the cryptsetup/crypttab rework (that one is scary, and I didn't read it in details yet): The "last column" of crypttab can contain space, meaning that everything past the fourth column will be treated as one column (the options column). Did you take that into account? I don't think so. I know that crypttab is ugly and scary, and I was planning to replace it with something sane, I just never got around to actually doing it.
On Fri, Jul 23, 2010 at 7:29 AM, Thomas Bächler <thomas@archlinux.org> wrote:
Am 17.07.2010 16:24, schrieb Victor Lowther:
Sorry for taking so long. Apart from squashing the trivial bashifications, all patches I didn't comment on are ACKed.
That is cool. I went ahead and reworked/squashed everthing I considered a "trivial bashification" into 6 or 7 patches after taking into account the feedback from the ML. The result is the bashification-redux branch @ git://fnordovax.org/~victor/arch-initscripts -- it is now 16 patches instead of 48, but the end result is exactly the same. I can post it to the ML if you like.
I looked over the new branch and found it much more readable. And although some of the patches are not 100% clean yet (at least one contains a trivial change that wasn't advertised in the commit), I like it.
Just a remark about the cryptsetup/crypttab rework (that one is scary, and I didn't read it in details yet): The "last column" of crypttab can contain space, meaning that everything past the fourth column will be treated as one column (the options column). Did you take that into account? I don't think so.
After looking over it again, you are correct -- split into array does not do The Right Thing here. It will be easy to fix -- by the time you read this it will have been fixed and rebased back into bashification-redux.
I know that crypttab is ugly and scary, and I was planning to replace it with something sane, I just never got around to actually doing it.
Am 23.07.2010 22:58, schrieb Victor Lowther:
After looking over it again, you are correct -- split into array does not do The Right Thing here. It will be easy to fix -- by the time you read this it will have been fixed and rebased back into bashification-redux.
It is, thanks. I'll have to look at that in more detail when I am less tired. When that's done, I can release new initscripts so we can do loooong testing.
On Fri, Jul 23, 2010 at 5:51 PM, Thomas Bächler <thomas@archlinux.org> wrote:
Am 23.07.2010 22:58, schrieb Victor Lowther:
After looking over it again, you are correct -- split into array does not do The Right Thing here. It will be easy to fix -- by the time you read this it will have been fixed and rebased back into bashification-redux.
It is, thanks. I'll have to look at that in more detail when I am less tired. When that's done, I can release new initscripts so we can do loooong testing.
How goes the rest of the review?
Am 31.07.2010 18:44, schrieb Victor Lowther:
On Fri, Jul 23, 2010 at 5:51 PM, Thomas Bächler <thomas@archlinux.org> wrote:
Am 23.07.2010 22:58, schrieb Victor Lowther:
After looking over it again, you are correct -- split into array does not do The Right Thing here. It will be easy to fix -- by the time you read this it will have been fixed and rebased back into bashification-redux.
It is, thanks. I'll have to look at that in more detail when I am less tired. When that's done, I can release new initscripts so we can do loooong testing.
How goes the rest of the review?
I only wanted to have a closer look at the cryptsetup patch before I pull this into master. But as you noticed, I didn't take the time to do so yet.
On Sat, Jul 31, 2010 at 11:54 AM, Thomas Bächler <thomas@archlinux.org> wrote:
Am 31.07.2010 18:44, schrieb Victor Lowther:
On Fri, Jul 23, 2010 at 5:51 PM, Thomas Bächler <thomas@archlinux.org> wrote:
Am 23.07.2010 22:58, schrieb Victor Lowther:
After looking over it again, you are correct -- split into array does not do The Right Thing here. It will be easy to fix -- by the time you read this it will have been fixed and rebased back into bashification-redux.
It is, thanks. I'll have to look at that in more detail when I am less tired. When that's done, I can release new initscripts so we can do loooong testing.
How goes the rest of the review?
I only wanted to have a closer look at the cryptsetup patch before I pull this into master. But as you noticed, I didn't take the time to do so yet.
No problem. Anything I can do to help?
participants (2)
-
Thomas Bächler
-
Victor Lowther