Skip to content

feat: v1#1

Merged
tranhl merged 7 commits intomainfrom
feat/v1
Mar 27, 2024
Merged

feat: v1#1
tranhl merged 7 commits intomainfrom
feat/v1

Conversation

@tranhl
Copy link
Collaborator

@tranhl tranhl commented Mar 4, 2024

Stack

kevgo

This comment was marked as resolved.

@tranhl

This comment was marked as resolved.

kevgo

This comment was marked as resolved.

@tranhl

This comment was marked as resolved.

@kevgo

This comment was marked as resolved.

@github-advanced-security

This comment was marked as resolved.

@tranhl tranhl force-pushed the feat/v1 branch 3 times, most recently from 6019fa2 to ab12c3d Compare March 7, 2024 13:08
@tranhl

This comment was marked as resolved.

kevgo

This comment was marked as resolved.

const perennialBranches = [
...explicitBranches,
...repoBranches.filter((branch) =>
perennialRegex ? RegExp(perennialRegex).test(branch) : false

Check failure

Code scanning / CodeQL

Regular expression injection

This regular expression is constructed from a [GitHub Actions user input](1).
@tranhl

This comment was marked as resolved.

@tranhl tranhl requested a review from kevgo March 23, 2024 14:20
Copy link
Contributor

@kevgo kevgo left a comment

Choose a reason for hiding this comment

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

This is a really nice solution now! I have really only one major suggestion around (optionally) omitting the stack visualization when there is only one branch in it.

@tranhl tranhl force-pushed the feat/v1 branch 2 times, most recently from 1d2ab9d to 080655e Compare March 24, 2024 08:35
@tranhl tranhl requested a review from kevgo March 24, 2024 08:44
@tranhl
Copy link
Collaborator Author

tranhl commented Mar 24, 2024

@kevgo Ready for another round of review 😊. To summarise what I've changed:

  • I've taken the liberty to resolve a lot of comments to clean up the PR
    • On a similar note I've squashed the commits as well to tidy things up
  • Removed the confusing mention of Dependabot in the check-dist workflow
  • Renamed the job ID of the branch stack action to branch-stack
  • Created a reusable action for setting up the environment for each job, called setup-env. This is now called right after actions/checkout
  • Renamed the linting.yml workflow to ci.yml, and added a job to run unit tests
  • Separated the action's dependencies from devDependencies to dependencies
  • Removed the template action input from action.yml as it's no longer used. Turns out I mistakenly removed the perennial-regex input too, which I've restored
  • Used an alert instead of an emoji to warn users about adding content between the anchor tag and visualization

There's a couple of review conversations that are still unresolved, as they require further input from you. Keen to hear your thoughts on those! I'm super excited at how things are coming along as well, we're very close I think! 🚀

Copy link
Contributor

@kevgo kevgo left a comment

Choose a reason for hiding this comment

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

I think this is ready to go. I have only nitpicks left 🙂

@tranhl tranhl merged commit fd2156e into main Mar 27, 2024
@tranhl tranhl deleted the feat/v1 branch March 27, 2024 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants