Skip to content

Conversation

@FidelusAleksander
Copy link
Contributor

@FidelusAleksander FidelusAleksander commented Jun 6, 2025

Changes

This pull request enhances the .github/workflows/finish-exercise.yml workflow by introducing a new optional but recommended input parameter, exercise-title, and integrating it to provide more personalized and descriptive output.

The exercise-title should be added not only when calling the start-exercise workflow (as it is already) but also infinish-exercise workflow, ideally with consistency.

How to adapt

When bumping versions of finish-exercise workflow we will have to remember to add a new parameter exercise-title

  finish_exercise:
    name: Finish Exercise
    needs: [find_exercise, post_review_content]
    uses: skills/exercise-toolkit/.github/workflows/finish-exercise.yml@<next-tag>
    with:
      issue-url: ${{ needs.find_exercise.outputs.issue-url }}
      exercise-title: "Getting Started with GitHub Copilot" ## new

Potential future improvements could point to reading this value from a configuration file in both workflows

Checklist

  • I have added or updated appropriate labels to this PR
  • I have tested my changes
  • I have updated the documentation if needed

Copilot AI review requested due to automatic review settings June 6, 2025 13:47
@github-actions github-actions bot added the workflows Changes to reusable workflows label Jun 6, 2025

This comment was marked as outdated.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new required exercise-title input to the finish-exercise workflow and integrates it into the output steps.

  • Introduce exercise-title as a required workflow input
  • Expose the input via EXERCISE_TITLE env var for social text
  • Pass exercise_title into the template variables for the final comment
Comments suppressed due to low confidence (4)

.github/workflows/finish-exercise.yml:10

  • [nitpick] The new exercise-title input is not documented at the top of the workflow file. Consider adding a brief header comment or updating the README to explain this required parameter.
exercise-title:

.github/workflows/finish-exercise.yml:10

  • Introducing a required exercise-title input is a breaking change; consider providing a default value or marking it optional to avoid breaking existing callers.
exercise-title:

.github/workflows/finish-exercise.yml:49

  • There’s no test or example validating that EXERCISE_TITLE is correctly set and used in the script; consider adding a workflow test or example job to cover this new behavior.
env:

.github/workflows/finish-exercise.yml:107

  • [nitpick] The input name uses hyphens (exercise-title), but the template var uses snake_case (exercise_title), which may confuse consumers. Consider aligning naming conventions.
exercise_title: ${{ inputs.exercise-title }}

@FidelusAleksander FidelusAleksander added breaking Breaking changes bump-minor Corresponds to minor SemVer release and removed breaking Breaking changes labels Jun 6, 2025
Copy link
Member

@chriswblake chriswblake left a comment

Choose a reason for hiding this comment

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

Let's see if we can make a version that isn't a breaking change. :)

@FidelusAleksander FidelusAleksander changed the title feat!:add exercise-title required parameter to finish-exercise workflow feat: add exercise-title optional parameter to finish-exercise workflow Jun 9, 2025
@FidelusAleksander FidelusAleksander removed the breaking Breaking changes label Jun 9, 2025
@FidelusAleksander FidelusAleksander merged commit 3da6262 into main Jun 9, 2025
6 checks passed
@FidelusAleksander FidelusAleksander deleted the update-finish-exercise-experience branch June 9, 2025 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bump-minor Corresponds to minor SemVer release workflows Changes to reusable workflows

Development

Successfully merging this pull request may close these issues.

3 participants