Skip to content
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

GitHub Release task suggestions #8922

Merged
merged 3 commits into from
Nov 30, 2018
Merged

Conversation

davidstaheli
Copy link
Contributor

Can you please consider these tweaks to the new GitHub Release task?

  1. Tweaks to strings
  2. Capitalize "GitHub"
  3. Remove input aliases. Feel free to reject this if it's too disruptive. But making the input name match the alias name makes docs generation nicer, starts clean where aliases aren't necessary before the task ships, and keeps names the same between JSON and YAML.
  4. Change the service connection input to allow only PAT and OAuth schemes. The "Token" scheme isn't supported by tasks because we have to keep the token private from the agents. :( See below:
"type": "connectedService:github:OAuth,PersonalAccessToken",

Copy link
Contributor

@mdmdakbar mdmdakbar left a comment

Choose a reason for hiding this comment

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

Thanks David for taking your time to review it.

Tasks/GithubReleaseV0/README.md Outdated Show resolved Hide resolved
"Create": "Create",
"Edit": "Edit",
"Discard": "Discard"
"create": "Create",
Copy link
Contributor

Choose a reason for hiding this comment

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

We used this value as $(action) in instanceNameFormat property below. So that task name can be modified according to action. For e.g. Create GitHub Release or Edit GitHub Release. But Lowercasing the options will lead to create GitHub Release or edit GitHub Release.

Copy link
Contributor Author

@davidstaheli davidstaheli Nov 28, 2018

Choose a reason for hiding this comment

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

@mdmdakbar, ah... I see what you mean. To the user, these values are case-insensitive when used in their YAML pipelines. Most GitHub users will use the typical YAML camel-casing and lowercase the the action value on their own, like this:

- task: GithubRelease@0
  inputs:
    action: create
    gitHubConnection: myConnection

Since most people will lowercase it in their YAML (no matter what casing is used in task.yaml), what would you think about changing instanceNameFormat to have the action at the end, like this?

  • GitHub release (create)
  • GitHub release (edit)
  • GitHub release (discard)

Tasks/GithubReleaseV0/task.json Outdated Show resolved Hide resolved
Tasks/GithubReleaseV0/task.json Outdated Show resolved Hide resolved
Tasks/GithubReleaseV0/operations/Utility.ts Show resolved Hide resolved
@davidstaheli
Copy link
Contributor Author

Since GitHub documentation refers to deleting a release, this PR also proposes that we use delete as the action name instead of discard.

@davidstaheli
Copy link
Contributor Author

Hi @mdmdakbar. I hope I've resolved everyone's feedback on this. I'll merge it based on your approval, but please let me know if I can help change anything later. Thanks very much for considering this PR!

@davidstaheli davidstaheli merged commit dd81e65 into master Nov 30, 2018
@davidstaheli davidstaheli deleted the users/davidstaheli/ghrelease branch November 30, 2018 12:08
@mdmdakbar mdmdakbar restored the users/davidstaheli/ghrelease branch December 12, 2018 14:17
mdmdakbar pushed a commit that referenced this pull request Dec 13, 2018
* GitHubRelease suggestions

* Updates for PR feedback

* Suggestion: use "delete" instead of "discard" to match GitHub wording
mdmdakbar added a commit that referenced this pull request Dec 13, 2018
* GitHub Release task suggestions (#8922)

* GitHubRelease suggestions

* Updates for PR feedback

* Suggestion: use "delete" instead of "discard" to match GitHub wording

* Fixing indentation issue in readme file (#9050)

* renaming folder name to match with make-options json (#9061)

* renaming folder name to match with make-options json

* Adding telemetry points for the GitHub release task (#9068)

* Adding telemetry points

* Making few more changes to support case insensitive action input and failing the when invalid action input is provided

* Modifyingstrings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants