Skip to content

Lint match extension name and directory #42

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 19, 2025
+16 −3
Merged

Lint match extension name and directory #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
24da094
Lint match extension name and directory
dotNomad Mar 19, 2025
4c76cef
Improve paths-filter comments
dotNomad Mar 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/actions/lint-extension/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,16 @@ runs:
using: "composite"

steps:
- name: Check extension name matches directory
run: |
MANIFEST_NAME=$(jq -r '.extension.name' < ./extensions/${{ inputs.extension-name }}/manifest.json)
if [ ${{ inputs.extension-name }} != $MANIFEST_NAME ]; then
echo "Error: Extension name, directory, and manifest.json mismatch"
echo "Extension '${{ inputs.extension-name }}' must be in the folder '/extensions/${{ inputs.extension-name }}'" and have the name '${{ inputs.extension-name }}' in the manifest.json.
Copy link
Contributor

@cgraham-rs cgraham-rs Mar 19, 2025

Choose a reason for hiding this comment

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

I had a little trouble retracing where exactly inputs.extension-name came from. It looks like this is the filter name a human manually typed into the dorny/paths-filter step.

Some potential improvements:

  • Be explicit in informing the user the filter as the source of the name in case that is actually where a typo exists. We already have code comments that I think are adequate in educating the user the filter has to match the extension.
  • This is extremely nit-picky, is there any areas of improvement for how we refer to these? Referring to extension name in the workflow, but that's really a filter name, and there's also a literal name: field in the manifest. Minor opportunity for confusion.
  • Go out of our way to further help the user by providing a verbose diff so it's entirely clear which of the things aren't matching.

Copy link
Collaborator Author

@dotNomad dotNomad Mar 19, 2025

Choose a reason for hiding this comment

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

It looks like this is the filter name a human manually typed into the dorny/paths-filter step

This is correct. If the extension name is typo-ed the first line of this will fail saying that ./extensions/${{ inputs.extension-name}}/manifest.json doesn't exist:

MANIFEST_NAME=$(jq -r '.extension.name' < ./extensions/${{ inputs.extension-name }}/manifest.json)

That will be the first clue.

Once that passes then the check is if the manifest has the right name. Those in tandem check the filter, directory, and manifest name all together.

I added improved comments for the filter name in: 4c76cef

I want to avoid commenting in the lint action itself that the name comes from the filter name since that isn't necessarily true for the complex tasks.

Copy link
Collaborator Author

@dotNomad dotNomad Mar 19, 2025

Choose a reason for hiding this comment

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

I'm going to get this merged, but we can continue the discussion and follow-up with any other improvements!

exit 1
fi
shell: bash

# Ensures that the manifest.json for the given extension name
# contains all the required fields for the rest of the release workflow
- run: |
Expand Down
9 changes: 6 additions & 3 deletions .github/workflows/extensions.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ jobs:
id: changes
with:
# Adding a new extension that has a directory that can be TARed?
# Add a new line here with the name of the extension and directory path
# Be sure the extension name and directory have the same name
# Add a new line here with the name of the extension as the name of
# the filter and the value as the extension's directory path
# Be sure the extension name and directory name are the same
# e.g. `extension-name: extensions/extension-name/**`
filters: |
reaper: extensions/reaper/**
Expand Down Expand Up @@ -76,8 +77,10 @@ jobs:
pull-requests: read
outputs:
# Adding a new extension with a complex build process?
# Add a new line here with the name of the extension step output variable below
# Add a new line here with the name of the extension as the name of the
# filter and the step output variable below
# e.g. `extension-name: ${{ steps.changes.outputs.extension-name }}`
# Be sure the extension name and directory name it is in are the same
publisher-command-center: ${{ steps.changes.outputs.publisher-command-center }}

steps:
Expand Down