Skip to content
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

Use javascript-action instead of platform dependent Shell commands #36

Merged
merged 1 commit into from
Apr 10, 2021

Conversation

juergenhoetzel
Copy link
Contributor

This change allows to execute github installation test in a platform-agnostic way (required to merge #35 by @ikappak)

Instead of a simple "test -x eldev" test, eldev is actually
bootstrapped again to verify that the net-installation was indeed
successful.

The previous shell tests also use the hardcoded path
https://raw.github.com/doublep/eldev/master/bin/eldev to download the
eldev scripts. So CI will not test changes introduced in the PRs.

The utility function getRawUrl generates the correct raw download
URL depending on the context (pushing to master or PR) and also adds a
platform dependent suffix (.bat on Windows) to the script name to make
the script download/testing platform-agnostic.

@juergenhoetzel
Copy link
Contributor Author

I have testet the functionality in dummy-repo https://github.com/juergenhoetzel/github-actions-test/pull/3

@doublep
Copy link
Collaborator

doublep commented Apr 10, 2021

Looks fine (I will trust you that it works like described, my JS and GitHub knowledge is limited). Please squash the two commits, they essentially do one thing together. And also a minor nitpick: please reformat if (...) do_something on two lines. I occasionally use this myself in Lisp when it is pure evaluation, but dislike single-line formatting in all other cases, and especially when condition is not simple.

Instead of a simple "test -x eldev" test, eldev is actually
bootstrapped again to verify that the net-installation was indeed
successful.

The previous shell tests also used the hard-coded path
https://raw.github.com/doublep/eldev/master/bin/eldev to download the
eldev scripts. So CI newer tested changes introduced in PRs.

The utility function getRawUrl generates the correct raw download
URL depending on the context (pushing to master or PR) and also adds a
platform dependent suffix (.bat on Windows) to the script name to make
the script download/testing platform-agnostic.
@juergenhoetzel juergenhoetzel changed the title Use javascript-action instead of platform dependend Shell commands Use javascript-action instead of platform dependent Shell commands Apr 10, 2021
@juergenhoetzel
Copy link
Contributor Author

Looks fine (I will trust you that it works like described, my JS and GitHub knowledge is limited). Please squash the two commits, they essentially do one thing together. And also a minor nitpick: please reformat if (...) do_something on two lines. I occasionally use this myself in Lisp when it is pure evaluation, but dislike single-line formatting in all other cases, and especially when condition is not simple.

Thanks for your quick and helpful response. I just squashed and rebased the commits.

@doublep doublep merged commit 6357448 into emacs-eldev:master Apr 10, 2021
@doublep
Copy link
Collaborator

doublep commented Apr 10, 2021

Merged, thank you.

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.

2 participants