Skip to content

Have automatic tagging as an option#35

Merged
miguelbemartin merged 3 commits intomasterfrom
tag-worklow-optional
Nov 27, 2020
Merged

Have automatic tagging as an option#35
miguelbemartin merged 3 commits intomasterfrom
tag-worklow-optional

Conversation

@ychartois
Copy link
Contributor

@ychartois ychartois commented Nov 26, 2020

Not everybody would like to have automatic tagging as it's tied to personal workflow. It perfect when your workflow follow the: 1 task = 1 PR, but it's not always the case.
By default the automatic tagging is turned off, if you want to enable it, you have to add it to your teamwork.yml like:

        with:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          TEAMWORK_URI: ${{ secrets.TEAMWORK_URI }}
          TEAMWORK_API_TOKEN: ${{ secrets.TEAMWORK_API_TOKEN }}
          AUTOMATIC_TAGGING: false

Fixes #33

toffentoffen
toffentoffen reviewed Nov 27, 2020
View reviewed changes
Copy link
Contributor

@toffentoffen toffentoffen left a comment

Choose a reason for hiding this comment

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

Small doubt to discuss. In general LGTM

src/main.sh
# Ensure env vars and args exist
ensure::env_variable_exist "GITHUB_REPOSITORY"
ensure::env_variable_exist "GITHUB_EVENT_PATH"
ensure::total_args 3 "$@"
Copy link
Contributor

@toffentoffen toffentoffen Nov 27, 2020

Choose a reason for hiding this comment

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

What do you think about checking if TEAMWORK_API_TOKEN exists as well? Isn't it mandatory as well?

Copy link
Contributor

@miguelbemartin miguelbemartin Nov 27, 2020

Choose a reason for hiding this comment

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

I think we don't need to check it because it's already defined as mandatory in the YML file.

miguelbemartin
miguelbemartin approved these changes Nov 27, 2020
View reviewed changes
Copy link
Contributor

@miguelbemartin miguelbemartin left a comment

Choose a reason for hiding this comment

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

Code looks good to me. I didn't test locally.

@ychartois
Copy link
Contributor Author

ychartois commented Nov 27, 2020

I did test but might have miss something... anyway, the "Ensure this projects runs" is stuck since yesterday...

@miguelbemartin
Copy link
Contributor

miguelbemartin commented Nov 27, 2020

I did test but might have miss something... anyway, the "Ensure this projects runs" is stuck since yesterday...

This is because this only runs when:

on:
  pull_request:
    types: [opened, closed]
  pull_request_review:
    types: [submitted, dismissed]

so if you update the PR, the workflow to test the action doesn't run again.

@miguelbemartin
Copy link
Contributor

miguelbemartin commented Nov 27, 2020

@ychartois feel free to merge this PR.

Don't forget to add a "new release" after PR has been merged.

@miguelbemartin miguelbemartin merged commit f4d4ec1 into master Nov 27, 2020
@miguelbemartin miguelbemartin deleted the tag-worklow-optional branch November 27, 2020 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@toffentoffen toffentoffen toffentoffen left review comments

@miguelbemartin miguelbemartin miguelbemartin approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ychartois @miguelbemartin @toffentoffen