Zsh Mailing List Archive
Messages sorted by:
Re: [PATCHv1] [long] improvements to limit/ulimit API and doc
- X-seq: zsh-workers 47666
- From: Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>
- To: Zsh hackers list <zsh-workers@xxxxxxx>
- Subject: Re: [PATCHv1] [long] improvements to limit/ulimit API and doc
- Date: Fri, 27 Nov 2020 17:16:38 +0000
- Archived-at: <https://zsh.org/workers/47666>
- Archived-at: <http://www.zsh.org/sympa/arcsearch_id/zsh-workers/2020-11/20201127171638.GE26720%40tarpaulin.shahaf.local2>
- Authentication-results: zsh.org; iprev=pass (out3-smtp.messagingengine.com) smtp.remote-ip=220.127.116.11; dkim=pass header.d=daniel.shahaf.name header.s=fm2 header.a=rsa-sha256; dkim=pass header.d=messagingengine.com header.s=fm1 header.a=rsa-sha256; dmarc=none header.from=daniel.shahaf.name; arc=none
- Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= daniel.shahaf.name; h=date:from:to:subject:message-id:references :mime-version:content-type:content-transfer-encoding :in-reply-to; s=fm2; bh=RZsiBHW1uD525Bnka6G8i46lcrjPhC6refC1Ucgj IAg=; b=bKEb8g/JBFTNSmasBl1j9cSyiGri7hk1ORbj+FSB0vqAmfiMyorEajEC Bsd9MI7crHJK2e4uCQxhI26lF0CKAchsLjrB/cyYdPigswFo0PEiTCYa0WyhZi7y EJQlrZypzixT+rHw//UJ3YNWPdUoFeXhXojyWPhpZH3u7NpmhtfE87myqxjaMR1o Zoxwzf7MQG41Bu3J+lGSO+CP+4KhleLPPbZ3cbeMrdAe8m2gxRMAdvlilaeziA+N 6TR8VMR9vxPWLWqPSLDoRKo23mxCKVp9AjwoSUK2RzCQm7m/+O5p4XuSYopcxKPv R36UqqjB0O6dYkiDw2mj7COj5Wn9qg==
- Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=RZsiBHW1uD525Bnka6G8i46lcrjPhC6refC1UcgjI Ag=; b=W5Z2/RcXZ6ItwKnxkQftk5ImP6g3w5uznwvgl/WlGoKsCXkCRVKag+6lU GpRZEiZ1gr+X4PINoKhbRb8vu525aas74QsClW0n17aZgGNiyi/riK3ynEf24Zb1 6bJCQ7ZCB7pT3DkmsiRmFCSwBf3Hd8wVA95b9mcVd/mLlGXNAfRZO6cb7mhe/Y0t r5Y0iyHcmr1guk/QdKKtBE2KgWfC1zSRA//kZhQBVAY0431J/vO8NdLqH4y/VAyK MFnYLyvu+1nr5PZQLIzm7qMZjMh2F29rP15B9BInIDx2XTTbDTiZ8g0D7Mda5rOK Ae7Gp4fPt/1maH1fjYf0I6giLBUDw==
- In-reply-to: <firstname.lastname@example.org>
- List-archive: <http://www.zsh.org/sympa/arc/zsh-workers>
- List-help: <mailto:email@example.com?subject=help>
- List-id: <zsh-workers.zsh.org>
- List-owner: <mailto:firstname.lastname@example.org>
- List-post: <mailto:email@example.com>
- List-subscribe: <mailto:firstname.lastname@example.org?subject=subscribe%20zsh-workers>
- List-unsubscribe: <mailto:email@example.com?subject=unsubscribe%20zsh-workers>
- References: <firstname.lastname@example.org> <20201125003512.GC17978@tarpaulin.shahaf.local2> <email@example.com>
- Sender: zsh-workers-request@xxxxxxx
Stephane Chazelas wrote on Wed, Nov 25, 2020 at 06:44:43 +0000:
> 2020-11-25 00:35:12 +0000, Daniel Shahaf:
> > > A few issues addressed:
> > >
> > This patch is over 1k lines long and makes multiple independent changes.
> > Could you please split this into one patch per logical change? That
> > would make it easier to review, both now, and in the future should
> > a regression be bisected to it.
> Hi Daniel,
> thanks for having looking into it.
> They're not independent, except maybe for the "limit"/"unlimit"
> now back in csh which is a 5 code line change.
> Chunks of the code and doc have been rewritten to address
> those. Separating out the changes would mean rewriting the code
> several times which would be more effort for me and reviewers,
> mostly wasted if we're only keeping the last iteration.
How so? In v2 2/2's log message I see, for starters, several bullet
points about documentation-only bugs, and in the .c diff the
showlimitvalue() stands out as _prima facie_ independent of the rest.
I don't see how _splitting_ those involves a _rewrite_. Splitting is
fairly easy,¹ and I don't think it'd be a "waste" of time. On the
contrary, I think it'd be a O(1) effort for O(N) gain (split it once and
then, even years from now, anyone who reviews the history will have a
easier time). Moreover, smaller diff can help ensure that each change
is tested and help catch unintentional behaviour changes (always a risk
I do agree that a single logical change that happens to fix a dozen
interdependent bugs needn't be split, but I don't see how that's the
case here. Enlighten me ☺
> But maybe before looking in detail at the code, we can discuss
> whether the change in API makes sense.
Seems to make sense, going by the log message. Really, the only part
that jumped out at me was changing the behaviour of "KB" on input from
error to "1000 bytes".
¹ Stephane: I elaborated offlist recently on splitting.
Messages sorted by: