feat: Add ability to run commands at given path#536
feat: Add ability to run commands at given path#536AtzeDeVries wants to merge 4 commits intomagefile:masterfrom
Conversation
Sometimes a command needs to run at a given path. The current `sh` package does not have this ability. This PR adds the feature. It adds "At" to all the run functions where you can supply it with a path were it needs to run. The functions are added under a new name so this change is backwards compatable. Signed-off-by: Atze de Vries <atze.wiebe.de.vries@coop.no>
Signed-off-by: Atze de Vries <atze.wiebe.de.vries@coop.no>
AtzeDeVries
force-pushed
the
add-at-to-sh
branch
from
5ab52ea to
f3b80ef
Compare
michel-laterman
left a comment
michel-laterman
left a comment
There was a problem hiding this comment.
Overall, it looks good.
I mostly have nitpicks
| t.Errorf("expected ran to be true but was false.") | ||
| } | ||
| if out.String() != fmt.Sprintf("%s\n", pwd) { | ||
| t.Errorf("expected %s, got %q", fmt.Sprintf("%s\n", pwd), out) |
There was a problem hiding this comment.
Why is fmt.Sprintf("%s\n", pwd) used here?
There was a problem hiding this comment.
because we are running the command "pwd" which should result in the correct pwd, which is set to be "/". So that is what we expect to get in return.
There was a problem hiding this comment.
Why not use t.Errorf("expected %s, got %q", pwd, out) or t.Errorf("expected %s\n, got %q", pwd, out) (if a newline is needed)?
| t.Errorf("expected ran to be true but was false.") | ||
| } | ||
| if out.String() != fmt.Sprintf("%s\n", currentWd) { | ||
| t.Errorf("expected %s, got %q", fmt.Sprintf("%s\n", currentWd), out) |
There was a problem hiding this comment.
Why is fmt.Sprintf("%s\n", currentWd) used?
There was a problem hiding this comment.
we run ExecAt with pwd "", we should test that this returns the actuall current path.
Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com>
Signed-off-by: Atze de Vries <atze.wiebe.de.vries@coop.no>
AtzeDeVries
changed the title
|
@natefinch Is this something you would like to have? If so, then i will keep the changes in sync with the current work you are doing. |
2 participants
Sometimes a command needs to run at a given path. The current
shpackage does not have this ability. This PR adds the feature. It adds "At" to all the run functions where you can supply it with a path were it needs to run. The functions are added under a new name so this change is backwards compatable.