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

feat: 🔨 better help text for checkForNewRelease() #32

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

techieshark
Copy link
Contributor

@techieshark techieshark commented Feb 21, 2024

What:
in scripts/requirements.js:

  1. code put inside an async function so we can await another function 2) checkForRelease() now awaited and moved to end (so its output falls
    at the end of the execution, just like presently

in scripts/release-check.js:

  1. we make checkForNewRelease an async function and switch over from promise syntax (see "releaseInfo.then()..." change) 2) introduce helper to fetch from github based on a subpath 3) add new function to get the commit of the main branch in github (this seems to fix a bug where previously "target_commitish" was "main" rather than a SHA) 4) update the logging in checkForNewRelease to use colors / icons similar to how the requirements.js works (in fact we pass in checkmark icons etc).

screenshots

When all is well:
image

If the SHA didn't match:

image

^ note, I hacked it to show that, hence why the SHA values actually do match in this example

AND note the blue help text:

image

What:
in scripts/requirements.js:
1) code put inside an async function so we can await another function
2) checkForRelease() now awaited and moved to end (so its output falls
  at the end of the execution, just like presently

in scripts/release-check.js:
1) we make checkForNewRelease an async function and switch over from promise syntax (see "releaseInfo.then()..." change)
2) introduce helper to fetch from github based on a subpath
3) add new function to get the commit of the main branch in github (this seems to fix a bug where previously "target_commitish" was "main" rather than a SHA)
4) update the logging in checkForNewRelease to use colors / icons similar to how the requirements.js works (in fact we pass in checkmark icons etc).
Copy link

@Osong-Michael Osong-Michael left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it locally, works fine for me. Maybe @lithrel could have some thing to add or he approves it.
Screenshot 2024-05-03 at 10 40 35

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

Successfully merging this pull request may close these issues.

3 participants