Paths

Table of Contentst

Differential D26640

Lua version of @shell
ClosedPublic
Actions

Authored by bapt on Oct 2 2020, 8:23 AM.

Tags

None

Referenced Files

	Unknown Object (File)
	Wed, Apr 17, 7:50 AM

	Unknown Object (File)
	Fri, Apr 12, 4:51 AM

	Unknown Object (File)
	Fri, Apr 12, 12:51 AM

	Unknown Object (File)
	Sun, Apr 7, 11:29 PM

	Unknown Object (File)
	Thu, Apr 4, 6:11 AM

	Unknown Object (File)
	Sun, Mar 31, 8:06 AM

	Unknown Object (File)
	Feb 8 2024, 12:42 PM

	Unknown Object (File)
	Dec 23 2023, 12:36 AM

View All 18 Files

Subscribers

Details

Reviewers

manu
mat

Group Reviewers

Commits

rP552938: Convert @shell from shell script to lua script

Summary

Bonus it manipulates less /etc/shell only adding a line if needed

Diff Detail

Repository

rP FreeBSD ports repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bapt created this revision.Oct 2 2020, 8:23 AM

Herald added 1 blocking reviewer(s): portmgr. · View Herald TranscriptOct 2 2020, 8:23 AM

Herald added a subscriber: mat. · View Herald Transcript

bapt requested review of this revision.Oct 2 2020, 8:23 AM

Harbormaster completed remote builds in B33946: Diff 77774.Oct 2 2020, 8:23 AM

Use variable when possible

Harbormaster completed remote builds in B33947: Diff 77775.Oct 2 2020, 8:25 AM

LGTM

Use a buffer when deleting which avoid some syscalls and a temporary file

Harbormaster completed remote builds in B33994: Diff 77879.Oct 5 2020, 8:43 AM

Even better thanks.

mat added inline comments.Oct 14 2020, 2:16 PM

Keywords/shell.ucl
35 ↗	(On Diff #77879)	Why `==` in the post-install and `~=` here?

kevans added a subscriber: kevans.Oct 14 2020, 2:18 PM

kevans added inline comments.

Keywords/shell.ucl
35 ↗	(On Diff #77879)	This one is rewriting all the lines that arent' the shell_path, the other one is bailing out as soon as we find shell_path

mat added inline comments.Oct 14 2020, 2:29 PM

Keywords/shell.ucl
35 ↗	(On Diff #77879)	Mmmm, I fail to see where this answers my question.

bapt added inline comments.Oct 14 2020, 3:05 PM

Keywords/shell.ucl
35 ↗	(On Diff #77879)	So on deinstall we bufferize the content of /etc/shells for all lines except the one we want to possibility remove. If the shell we are interested in is found we overwrite /etc/shells if not we just drop the buffer ;) on install this is this opposite we append our new shell only if we haven't already found it in existing /etc/shells

mat added inline comments.Oct 14 2020, 4:02 PM

Keywords/shell.ucl
35 ↗	(On Diff #77879)	No, I get your point about what post-install and pre-deinstall do, I did the first time around. What about my question?

mat added inline comments.Oct 14 2020, 4:37 PM

Keywords/shell.ucl
35 ↗	(On Diff #77879)	Maybe I was not clear enough, doing if <not> ( sonething ) is ok, but doing if <not> (something) else (something else) is an anti-pattern, if there is an else, the test must be a positive thing and the clauses switched, otherwise it gets hard on the brain.

Fix anti-pattern

bapt marked 5 inline comments as done.Oct 15 2020, 6:37 AM

bapt added inline comments.

Keywords/shell.ucl
35 ↗	(On Diff #77879)	Right ok, now I understand what you mean makes sense, let me fix that ;)

Harbormaster completed remote builds in B34183: Diff 78253.Oct 15 2020, 6:37 AM

mat accepted this revision.Oct 21 2020, 3:59 PM

This revision is now accepted and ready to land.Oct 21 2020, 3:59 PM

Closed by commit rP552938: Convert @shell from shell script to lua script (authored by bapt). · Explain WhyOct 22 2020, 6:39 AM

This revision was automatically updated to reflect the committed changes.

bapt marked an inline comment as done.

bapt added a commit: rP552938: Convert @shell from shell script to lua script.

Revision Contents
Changeset List

Path

Size

Packages

head/

Keywords/

51 lines

O5: Ports Framework

Diff 78574

head/Keywords/shell.ucl

Loading...