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

Rewrite in TS/JS #80

Closed
spenserblack opened this issue May 22, 2023 · 7 comments · Fixed by #81
Closed

Rewrite in TS/JS #80

spenserblack opened this issue May 22, 2023 · 7 comments · Fixed by #81
Assignees

Comments

@spenserblack
Copy link
Owner

spenserblack commented May 22, 2023

See #66 (comment)

@spenserblack spenserblack self-assigned this May 22, 2023
@spenserblack spenserblack linked a pull request May 22, 2023 that will close this issue
@jcbhmr
Copy link
Contributor

jcbhmr commented May 23, 2023

I know I mentioned this before, but I've since improved the "deno_wrapper" project: https://github.com/jcbhmr/deno_wrapper

Just use ./denow as though it were the true deno binary! Anyone who clones your repo won't need to install deno themselves; the ./denow will auto-install a local copy into the .deno folder.

For instance, GitHub Actions can be written using Deno, but how do you make sure deno is available on the GitHub Action runner? You can use ./denow (or ./denow.bat) as a proxy!

@jcbhmr
Copy link
Contributor

jcbhmr commented May 23, 2023

Won't it be longer code-wise? 🤔 Most of the stuff in this problem space is just going to be running bash commands in series, right? There's not a whole lot of string processing, output inspection, if checks, etc. There's only four spots where I can see logic that's not just a literal sequence of shell commands 😂

# When a job fails, you can re-run it with debug mode enabled. This is exposed
# to scripts via the ${{ runner.debug }} or $RUNNER_DEBUG variable. Here, we
# use the -z test to see if the $RUNNER_DEBUG exists. If so, we use the -x flag
# to print a '+ cmd arg1 arg2' of each command that's run in the script. This
# helps with debugging what commands and $VAR expansions are actually happening.
if [[ -z $RUNNER_DEBUG ]]; then
set -x
fi

# This is the default host that gh uses for clones and commands without a repo
# context (a .git folder). We use Bash string magic to get the github.com part
# from a full origin (no pathname) like https://github.com => github.com. The
# '#*//' operation removes '*//' from the start of the string. That's the
# 'https://' chunk. With that gone, we are left with 'github.company.com' or
# something similar.
export GH_HOST="${GITHUB_SERVER_URL#*//}"

# This is just good practice to clean up after yourself. It's not needed per-se.
# This is a one-off Actions runner that will delete every part of itself after
# completion. It's a good habit nonetheless.
trap 'rm -rf "$GIT_DIR"' SIGINT SIGTERM ERR EXIT

actions-wiki/src/clone.sh

Lines 101 to 108 in 1181a52

# If we are given 'dry-run: true', then we want to just print changes and stop
# without pushing. This is only used in testing right now.
if [[ $INPUT_DRY_RUN == true ]]; then
echo 'Dry run'
git remote show origin
git show
exit 0
fi

What I'm getting at is that I don't entirely know why this is being rewritten in JS/TS when it's just going to be:

const inputIgnore = core.getInput("ignore")
const inputToken = core.getInput("token")
process.env.GITHUB_TOKEN = inputToken
// More code

try {
  await $`gh auth setup-git`
  await $`git clone ${process.env.GITHUB_SERVER_URL}${inputRepository}.wiki.git ${process.env.GIT_DIR}`
  await fsPromises.writeFile(join(process.env.GIT_DIR, ".git/info/exclude"), inputIgnore)
  // More git shell commands
} catch {
  rimraf(process.env.GIT_DIR)
}

But

If there's some feature that you're planning on adding, that's cool and interesting! Like for instance auto preprocessing the markdown like https://github.com/impresscms-dev/strip-markdown-extensions-from-links-action/blob/main/src/index.ts ⬅ That's the cool stuff that needs Node.js to work well! Shell commands in a sequence... not really.

But that's my opinion. 🤷‍♀️

@spenserblack
Copy link
Owner Author

I find TS easier to contribute to, and also I think it's easier to write tests. That's my basic reasoning. Also, I think the language looks prettier 🙃

@jcbhmr
Copy link
Contributor

jcbhmr commented May 23, 2023

I find TS easier to contribute to, and also I think it's easier to write tests. That's my basic reasoning. Also, I think the language looks prettier 🙃

Sounds awesome! More tests is always better! 🎉

@jcbhmr
Copy link
Contributor

jcbhmr commented May 29, 2023

I know I mentioned this before, but I've since improved the "deno_wrapper" project: https://github.com/jcbhmr/deno_wrapper

Thought you might be interested to know that I have dog-fooded my own thing! 😊 https://github.com/jcbhmr/set-secrets-action

@spenserblack
Copy link
Owner Author

Interesting idea! I've repeated secrets among multiple repos sometimes, could be handy 👍

@jcbhmr
Copy link
Contributor

jcbhmr commented Jul 16, 2023

Turns out that https://github.com/Andrew-Chen-Wang/github-wiki-action needed the push to TS after all. Andrew-Chen-Wang/github-wiki-action#68 With Andrew-Chen-Wang/github-wiki-action#8 you need an AST parser, and JavaScript has Markdown parsers in spades. I used a modified https://github.com/jcbhmr/deno_wrapper script cliw to then run cli.ts with no build step with a negligible runtime cost (there's enough GH Actions overhead that 400ms to download Deno+npm stuff doesn't matter)

Just figured you might be interested 😉

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 a pull request may close this issue.

2 participants