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

Enable validation for jira issue status #33

Merged
merged 17 commits into from
Dec 20, 2020
Merged

Conversation

NasAmin
Copy link
Contributor

@NasAmin NasAmin commented Oct 16, 2020

Enables validation of jira issue status against a list of allowed statuses provided as an action input.
This implements #32
Please refer to #32 for more details.

@NasAmin
Copy link
Contributor Author

NasAmin commented Oct 16, 2020

@all-contributors Please review.
Apologies if I have missed anything and please let me know if this can be done in a better way.

@NasAmin NasAmin mentioned this pull request Oct 16, 2020
@NasAmin
Copy link
Contributor Author

NasAmin commented Nov 2, 2020

I'll appreciate if the maintainers can review this PR please

@rheaditi
Copy link
Contributor

rheaditi commented Nov 2, 2020

Hey @NasAmin! Thanks for your contribution & raising this PR! Really glad you found this action useful :)

We've had issues with the markdown/markup syntax not getting reproduced correctly in the GitHub PR description. Could you add some screenshots of this in a test repo with the working PR description? Unfortunately we only have this means of manual testing..

I don't have write access to this repo anymore, although @loftykhanna could help out here!


@loftykhanna: Could you have a 👀 on this PR, please?

@NasAmin
Copy link
Contributor Author

NasAmin commented Nov 3, 2020

@rheaditi Thanks for getting back to me.
The PR description issue, is this a general issue or are you talking about the markup this PR will be generating?
I have put the generated markup on issue #32
This is what my PR will generate:
image

If you are talking about the general PR description bit of jira-lint then I can look into that but it should probably be a separate PR.

@NasAmin
Copy link
Contributor Author

NasAmin commented Dec 3, 2020

@loftykhanna @maddhruv I would really appreciate if you can review and approve this PR and release it.
I have resolved merge conflicts ✔️

@rheaditi regarding the PR description issue, I am using the release version of this action and it is producing the PR descriptions just fine

image

PR title not matching the Jira title
image

src/main.ts Outdated Show resolved Hide resolved
Copy link

@loftykhanna loftykhanna left a comment

Choose a reason for hiding this comment

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

@NasAmin I tested your code, no comments are getting added after it fails.
@rheaditi Comment/suggestion might help in fixing this. Please fix and I will test again.

Also, a suggestion, can 'allowed_issue_statuses' be case insensitive?
image

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/utils.ts Outdated Show resolved Hide resolved
src/utils.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@rheaditi rheaditi left a comment

Choose a reason for hiding this comment

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

@NasAmin have a few minor comments.. Please take a look whenever you get time 🙂

@NasAmin
Copy link
Contributor Author

NasAmin commented Dec 9, 2020

@NasAmin I tested your code, no comments are getting added after it fails.
@rheaditi Comment/suggestion might help in fixing this. Please fix and I will test again.

Also, a suggestion, can 'allowed_issue_statuses' be case insensitive?
image

@loftykhanna Thanks for testing this. Not sure how I can easily test the PR comment bit if it fails but it might have been because the addComment() call didn't have an await. @rheaditi suggested that change which I have applied.

Regarding allowed_issue_statuses case insensitive, the statuses are passed to Jira really so if Jira supports case insensitive it should work otherwise we may not have a choice. But AFAIK, JQL is case insensitive. I just tested with the following
image

@NasAmin
Copy link
Contributor Author

NasAmin commented Dec 9, 2020

@rheaditi and @loftykhanna thank you both for reviewing my code. I have addressed all the comments (I think).
However, I am not sure how to test the PR comment if the issue status check fails. @loftykhanna Could you please test it again or let me know how to test it?

If there is a way to write some kind of automated test then that would be great for the future.

@loftykhanna
Copy link

@NasAmin Sure let me test and get back to you on this.

@loftykhanna
Copy link

@NasAmin Thank you for your contribution, Its working fine now 👍

@loftykhanna loftykhanna merged commit 0b90e54 into ClearTax:master Dec 20, 2020
@NasAmin NasAmin deleted the master branch January 16, 2021 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants