-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add ability to send metadata from issues #37
Conversation
I've added the extra check mentioned by @feerrenrut in nvaccess/addon-datastore-validation#8 |
Use NV Access repositories
d8202c4
to
12e5af0
Compare
I'm not sure if dropdowns in form issues are very accessible. Not the last time I tested. I point out this in case more than stable and beta channels are accepted. |
I tested these with NVDA, they seemed to work ok for me. They are announced as a |
There are several blockers with this:
For now, I'll continue to recommend that submitters create the JSON file locally, push to their fork, then open a PR against the |
It works here too with NVDA. It's similar to the Reviewers menu to request a review for a pull request and chosing one of the available reviewers. |
Anyway, when I request a review, NVDA doesn't reports the word selection and I think it's better. Maybe better to remove the placeholder. |
OK. |
I don't think we have any control over this, I think this is built-in (by GitHub) behavior. |
I understand. |
Hi @seanbudd I investigated this again trying to make easy for people to send add-ons. After some modifications in my fork of this repo, results are the following:
Notes:
Thanks @CyrilleB79 . You can create an issue at nvdaes store repo if you have some add-ons for further confirmation. |
I've marked this as ready for review. Time permitting, let me know if I should make more tests on my fork, for example, to use an application instead of a personal access token. |
For an example of this working, after removing the eMule add-on from my fork to allow automerge, see: |
@seanbudd Regarding labels, now it will be applied the addonSubmission label. You need to add it to this repo since it's not available by default, and if it's not available it won't be applied. |
Yes can you make it GitHub actions or the submitter of the issue? |
Hi Sean: the GitHub script action, used here, is intended to use JavaScript according to its documentation. In the checkMetadata workflow we use Javascript too, to check the files changed, though the script is inline and not in a separate file. So I think that it"s better to follow documentation of the GitHub script function. There are other methods to validate issue forms but not tested by me. I"ll make all suggested changes in a few hours.
|
@seanbudd I have addressed your review as follows:
I've tried other options following recommendations from internet, but finally this worked: https://github.com/orgs/community/discussions/26261 Please let me know if I should do anything else. |
Hi again, setting the author to github-actions doesn't work. I've tried another approach: using the event.sender.login and the noreply email address. |
Unfortunately seems that the author field has no effect on the sendJsonFile workflow. I don't know what to do with this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @nvdaes, this looks good to me.
I am going to make the minor changes and merge
Thanks Sean. I've read that GitHub doesn't allow to change PR authors
to avoid making inappropriate things which maybe attributed to a
different person. I don't know.
2023-03-07 21:24 GMT+01:00, Sean Budd ***@***.***>:
… Merged #37 into master.
--
Reply to this email directly or view it on GitHub:
#37 (comment)
You are receiving this because you were mentioned.
Message ID:
***@***.***>
|
Thanks, you will inform when this can be used.
2023-03-07 21:20 GMT+01:00, Sean Budd ***@***.***>:
… @seanbudd approved this pull request.
Hi @nvdaes, this looks good to me.
I am going to make the minor changes and merge
> +title: "[Submit add-on]: "
+labels: [autoSubmissionFromIssue]
+assignees:
+- nvaccess
+body:
+# Docs on using issue forms:
+#
https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/syntax-for-githubs-form-schema
+- type: markdown
+ attributes:
+ value: |
+ Submit an NVDA add-on to datastore. Provide download and source URL,
and indicate if it's a stable add-on or not.
+- type: input
+ id: download-url
+ attributes:
+ label: Download URL
+ description: The URL to download the add-on. It must start with 'https'
and end with 'nvda-addon'.
```suggestion
description: The URL to download the add-on. It must start with 'https'
and end with 'nvda-addon'. This URL should always download the same file of
the same add-on version.
```
> + const licenseUrlMd = "### License URL"
+ //
+ // collect variables from issue form
+ //
+ const body = context.payload.issue.body
+ const downloadUrl =
body.split(dlTitleMd)[1].split(header3Prefix)[0].trim()
+ core.setOutput('downloadUrl', downloadUrl)
+ const sourceUrl =
body.split(sourceUrlMd)[1].split(header3Prefix)[0].trim()
+ core.setOutput('sourceUrl', sourceUrl)
+ const releaseChannel =
body.split(channelMd)[1].split(header3Prefix)[0].trim()
+ core.setOutput('releaseChannel', releaseChannel)
+ const licenseName =
body.split(licenseMd)[1].split(header3Prefix)[0].trim()
+ core.setOutput('licenseName', licenseName)
+ const licenseUrl =
body.split(licenseUrlMd)[1].split(header3Prefix)[0].trim()
+ core.setOutput('licenseUrl', licenseUrl)
+}
can this get a new line at EOF
> + const licenseUrlMd = "### License URL"
+ //
+ // collect variables from issue form
+ //
+ const body = context.payload.issue.body
+ const downloadUrl =
body.split(dlTitleMd)[1].split(header3Prefix)[0].trim()
+ core.setOutput('downloadUrl', downloadUrl)
+ const sourceUrl =
body.split(sourceUrlMd)[1].split(header3Prefix)[0].trim()
+ core.setOutput('sourceUrl', sourceUrl)
+ const releaseChannel =
body.split(channelMd)[1].split(header3Prefix)[0].trim()
+ core.setOutput('releaseChannel', releaseChannel)
+ const licenseName =
body.split(licenseMd)[1].split(header3Prefix)[0].trim()
+ core.setOutput('licenseName', licenseName)
+ const licenseUrl =
body.split(licenseUrlMd)[1].split(header3Prefix)[0].trim()
+ core.setOutput('licenseUrl', licenseUrl)
+}
```suggestion
}
```
--
Reply to this email directly or view it on GitHub:
#37 (review)
You are receiving this because you were mentioned.
Message ID: ***@***.***>
|
The current planned procedure, consisting in sending json files via pull requests, has security issues as shown in PR #35
This adds an issue template and GitHub workflows to create pull requests and enable automerge from issues, checking that publisher is not modified and it's created based on the GitHub account of the issue creator. This publisher can edit metadata send by the corresponding person, for example to change the add-on channel from beta to stable once the add-on has been tested.