Skip to content

Replace cdd call with cd - #254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 19, 2020
Merged

Replace cdd call with cd - #254

merged 2 commits into from
Feb 19, 2020

Conversation

galfert
Copy link
Contributor

@galfert galfert commented Feb 18, 2020

In the dev script in package.json I noticed a call to cdd at the end. I didn't recognize that commend and found that I don't have such a command (on macOS).
I don't know if it's a custom script or something Linux specific, but I assume it's for changing back to the original directory after the script previously changed into packages/sockethub. I replaced it witch cd -, which does exactly that, but AFAIK is in the POSIX standard. So it should be available in most shells.

I also added it to the end of the start script as well, because that also changes directories.

@raucao
Copy link
Contributor

raucao commented Feb 18, 2020

May I suggest to use pushd and popd for that? I think it's much safer across environments and in scripts like that.

@gregkare
Copy link
Contributor

It turns out the cd - command is not needed, you can remove it and you are kept at the root of the repo. Only the subshell is moving to the packages/sockethub dir

@galfert
Copy link
Contributor Author

galfert commented Feb 18, 2020

Oh cool, didn't know that. Will remove it then.

The scripts are run in a subshell, so one is still in the repo's root
dir after running them.
@galfert galfert requested a review from silverbucket February 18, 2020 16:20
@silverbucket
Copy link
Member

Awesome, thanks. BTW can't figure out how to mark the PR as "approved" by me as I'm requested as a reviewer. Anyway, will merge and publish a new release.

@silverbucket silverbucket merged commit c730fbe into master Feb 19, 2020
@silverbucket
Copy link
Member

Nevermind about the publishing bit, :D forgot this was just for the repository and not for any of the packages.

@galfert galfert deleted the bugfix/remove_cdd branch February 19, 2020 10:52
@galfert
Copy link
Contributor Author

galfert commented Feb 19, 2020

BTW can't figure out how to mark the PR as "approved" by me as I'm requested as a reviewer.

In the "Files changed" tab there is a green button "Review changes" at the top right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants