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

Add auto commenting to PR with the preview url. #4

Merged
merged 4 commits into from
Sep 14, 2020

Conversation

daltonfury42
Copy link
Contributor

Totally loved your idea. Adding comment to the PR with the preview branch name:

image

Also I removed the 10 character limit on branch name.

Also, removed 10 character branch name limit

Update README.md

Add comment curl

Pass comment url to the action

Gate for toen

Update README.md

Update action.yml

Update README.md

Update entrypoint.sh

Update action.yml

Update entrypoint.sh

Update README.md

Update Dockerfile

Update entrypoint.sh

Update README.md

Update entrypoint.sh
Comment on lines +5 to +7
BRANCH_NAME=$1
AMPLIFY_COMMAND=$2
COMMENT_URL=$3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

a small readability improvement.

README.md Outdated Show resolved Hide resolved
Copy link
Owner

@yinlinchen yinlinchen left a comment

Choose a reason for hiding this comment

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

Hi @daltonfury42 I like this function! Thanks for the PR. Please see the comment and do you mind removing | cut -c-10 from the examples.md too? Thanks!

README.md Outdated
@@ -58,6 +59,7 @@ The following settings must be passed as environment variables as shown in the e
| `AWS_REGION` | The region where you created your bucket. Set to `us-east-1` by default. [Full list of regions here.](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html#concepts-available-regions) | `env` | **Yes** | `us-east-1` |
| `AmplifyAppId` | The unique ID for an Amplify app. For example, `d6a88fjhifqlks` | `secret env` | **Yes** | N/A |
| `BackendEnvARN` | The Amazon Resource Name (ARN) for a backend environment that is part of an Amplify app. | `secret env` | **Yes** | N/A |
| `GITHUB_TOKEN` | The GITHUB_TOKEN, required to post the comment with the preview URL | `github env` | **Yes** | N/A |
Copy link
Owner

Choose a reason for hiding this comment

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

This field is not required, right? Please change **Yes** to No

Copy link
Contributor Author

@daltonfury42 daltonfury42 Sep 14, 2020

Choose a reason for hiding this comment

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

I was thinking of keeping it as a required field, what do you think? That way, the comments will come as default.

Either we keep it as optional, or I can update entrypoint.sh accordingly. Let me know what is to be done.

Copy link
Owner

Choose a reason for hiding this comment

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

I think keep it as an optional field would be better. Let others to decide if they want to show the preview link to public by themselves, instead of showing it mandatory.

Copy link
Contributor Author

@daltonfury42 daltonfury42 Sep 14, 2020

Choose a reason for hiding this comment

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

I've made the change. 45b4f56

update example and examples - remove 10 character trim
Copy link
Owner

@yinlinchen yinlinchen left a comment

Choose a reason for hiding this comment

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

Nicely done! Thanks for adding this!

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.

2 participants