Lint - Categories in the properties.json file by aparna-ravindra · Pull Request #1099 · actions/starter-workflows

aparna-ravindra · 2021-09-16T05:41:16Z

Context

The categories listed in the .properties.json file of a template determine the section under which the template appears in the actions/new page.

What does this PR contain?

This PR aims at validating the categories field present in the .properties.json file for each of the templates and identifying that it contains at least one of the supported categories. The aim is to make the reviewer aware of such cases by commenting on the PR. The reviewer can then choose to recommend the right category for the template.

The list of recognised categories is maintained in the settings.json file and is shared in the comment with the PR owner.

This PR contains two workflows

Checkout the PR branch and validate categories for all the templates. Save the result in a workflow artifact.
Download the workflow artifact and comment on PR.

The comment on the PR appears like this: https://github.com/aparna-ravindra/starter-workflows/pull/42#issuecomment-920592073

tldr;
Two workflows were needed because the workflows running on the PR branch(from forked repo) do not have permission to comment on the issue. This has been documented here. In gist the doc says

combining pull_request_target workflow trigger with an explicit checkout of an untrusted PR is a dangerous practice that may lead to repository compromise.

At the same time it mentions:

If your workflow scenario simply requires commenting on the PR, but does not require a check out of the modified code, using pull_request_target is a logical shortcut.

Therefore the approach described in the doc has been followed.

bishal-pdMSFT · 2021-09-24T09:07:07Z

@@ -0,0 +1,37 @@
+require 'json'


Why not use validate-data script itself?

The reasons I plan on keeping the script separate

Modularity : The validate-data script already does multiple checks. We expect to add more checks in the validate-categories script, which will make validate-data bulky if combined.

Validate-Data is a mandatory check on PR, failing which will block PR merge. We intend Validate-categories to nudge the PR author with information on best practices for the values of the category field. There is no plan on failing the check.

Ah! I think I get it now. category validation is an optional thing and we only put a PR message. Seems like we discussed it long back.

@aparna-ravindra actually just re-thinking about it, currently we are only validating the template type category and not tech stack category. And if so, then we should fail the validation just like validate-data. And hence we can make this part of validate-data itself. Thoughts?

bishal-pdMSFT · 2021-09-24T09:52:45Z

+@allowed_categories = settings['allowed_categories']
+
+def validateCategories(categories)
+    return categories.nil? || (categories.is_a?(Array) && ! categories.select{|l| @allowed_categories.detect{|p| p.casecmp(l) == 0 } }.empty? )


Will array intersection achieve same result as select and detect?

Need this to be case-insensitive

bishal-pdMSFT · 2021-09-24T09:53:23Z

+@allowed_categories = settings['allowed_categories']
+
+def validateCategories(categories)
+    return categories.nil? || (categories.is_a?(Array) && ! categories.select{|l| @allowed_categories.detect{|p| p.casecmp(l) == 0 } }.empty? )


If categories is nil, then validate should fail

categories is an optional field. We do have a blank template today where there are no categories.

This seems troublesome to me. Blank is a special case we should handle is separately. For any other template, we can disallow missing categories.

bishal-pdMSFT · 2021-09-24T10:00:21Z

+
+result = []
+for folder in folders
+    files = Dir.entries(folder).select {|entry| File.file?(File.join(folder, entry)) && (File.extname(entry) == ".yaml" || File.extname(entry) == ".yml") }


Do we need to co-relate with workflow yml file? Or just simply look through all properties file?

The co-relation is taken care by the validate-data script.

No, I meant why do we need to first find the workflow file name and then look for properties.json for that? We can directly iterate through all properties.json and validate category.

bishal-pdMSFT · 2021-09-24T10:02:19Z

+# read-only repo token
+# no access to secrets
+on:
+  pull_request:


add on push event as well. ghes branch has direct pushes.

ghes branch is updated only through the sync-ghes workflow, I believe.
Also, we require a PR to comment if needed. Otherwise, the validation script is run, but the result is not shared anywhere.

Got it. So, essentially master branch is the gatekeeper for ensuring correct categories.

bishal-pdMSFT · 2021-09-24T10:02:50Z

@@ -0,0 +1,62 @@
+name: Validate Categories on PR


Please add a comment on what are the valid categories

bishal-pdMSFT · 2021-09-25T09:55:44Z

+      - name: Save Validation Output 
+        if: ${{steps.comment-format.outputs.content && steps.comment-format.outputs.content != '' }}
+        run: |
+          mkdir -p ./category_validation


This approach for publishing artifact is needed due to the reason that PR workflow won't have permission to write to PR in case of forked repos most probably. Isn't it?
Would it be easier to write a bot or as I mentioned in another comment to make category validation part of validate-data

aparna-ravindra added 3 commits September 16, 2021 10:54

validating categories

6c0cbc3

formatting

1966775

formatting

7bebee8

aparna-ravindra requested a review from a team as a code owner September 16, 2021 05:41

aparna-ravindra mentioned this pull request Sep 16, 2021

Adding category in the template property file #1092

Merged

22 tasks

Merge branch 'main' into categories-lint

847dbaf

ashwinsangem mentioned this pull request Sep 16, 2021

Starter workflow Symfony #1069

Merged

14 tasks done

aparna-ravindra added 2 commits September 20, 2021 11:21

Merge branch 'main' into categories-lint

3ce2c73

Merge branch 'main' into categories-lint

fc69912

bishal-pdMSFT reviewed Sep 24, 2021

View reviewed changes

Update validate-category-on-pr.yaml

13eda07

bishal-pdMSFT reviewed Sep 25, 2021

View reviewed changes

aparna-ravindra closed this Sep 28, 2021

nexoracontrol-ops mentioned this pull request Sep 28, 2026

# AI CLI 工具社区动态日报 2026-07-15 #3463

Open

Conversation

aparna-ravindra commented Sep 16, 2021

Context

What does this PR contain?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants