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

feat: implement 'working-directory' argument #120

Merged
merged 8 commits into from
Feb 22, 2023

Conversation

tyree731
Copy link
Contributor

@tyree731 tyree731 commented Dec 8, 2022

When running the action-release Github Action, occasionally one might need to perform auto-detection of associated commits in a directory other than the job's working directory, such as in the case where an action performs multiple checkouts. As Github Actions do not, by default, support the ability to run an action in any directory other than the job's working directory, this change implements an argument for specifying which directory to gather sentry release information from.

Implements #117 .

@tyree731 tyree731 requested a review from a team December 8, 2022 13:58
@armenzg
Copy link
Member

armenzg commented Dec 8, 2022

@tyree731 Thanks for the PR!

The original authors are not around.

Would you be able to test that the action works as you expect it to?
I wonder if testing the GH action against 53c985e2f27a102b54ddf6621affa847f7271648 would do the trick.

@armenzg armenzg removed the request for review from a team December 8, 2022 19:07
@tyree731 tyree731 closed this Dec 8, 2022
When running the action-release Github Action, occasionally one
might need to perform auto-detection of associated commits in a
directory other than the job's working directory, such as in the
case where an action performs multiple checkouts. As Github
Actions do not, by default, support the ability to run an action
in any directory other than the job's working directory, this
change implements an argument for specifying which directory to
gather sentry release information from.
@tyree731
Copy link
Contributor Author

tyree731 commented Dec 8, 2022

My github actions testing foo is weak so I accidentally closed this. I tested this here:

tyree731#1

And the existing tests work (with slight modification from my original change). For testing this with my change, would you like me to make another test that works off a subdirectory?

@armenzg armenzg reopened this Dec 9, 2022
@armenzg
Copy link
Member

armenzg commented Dec 9, 2022

My github actions testing foo is weak so I accidentally closed this. I tested this here:

tyree731#1

No worries

And the existing tests work (with slight modification from my original change). For testing this with my change, would you like me to make another test that works off a subdirectory?

Yes, please! That would be great!

@tyree731
Copy link
Contributor Author

Hey @armenzg , I added a test which probably works for testing this. Can you please enable running workflows for me? Thanks!

@tyree731
Copy link
Contributor Author

Ok, now I've got a working run:

https://github.com/tyree731/action-release/actions/runs/3677213354

@Amoodaa
Copy link

Amoodaa commented Jan 9, 2023

Will this get merged?

@armenzg
Copy link
Member

armenzg commented Jan 10, 2023

I added it to my to-do to look at this week. My apologies on the delay.

@Amoodaa
Copy link

Amoodaa commented Jan 10, 2023

A reminder to add in the README section, im happy to add it, but i dont wanna complicate the PRs
cc: @tyree731 @armenzg

@tyree731
Copy link
Contributor Author

Hi, sorry, lost track of this one. So, should I add the README before this gets merged?

@tyree731
Copy link
Contributor Author

Went ahead and added the README changes needed here. Let me know if there is anything else I need to do. Thanks!

@tyree731
Copy link
Contributor Author

@armenzg hi. Anything else I need to do here, or are we good to get this merged?

Copy link
Member

@armenzg armenzg left a comment

Choose a reason for hiding this comment

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

This is very good!
Thank you for the contribution. My apologies on my delayed feedback.

@armenzg armenzg merged commit 3b3f927 into getsentry:master Feb 22, 2023
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.

3 participants