search

emacs-eldev / eldev

Elisp development tool
https://emacs-eldev.github.io/eldev/
GNU General Public License v3.0
227 stars 17 forks source link

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

Closed juergenhoetzel closed 3 years ago

juergenhoetzel commented 3 years ago

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 commented 3 years ago

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

doublep commented 3 years ago

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.

juergenhoetzel commented 3 years ago

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 commented 3 years ago

Merged, thank you.