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 the optional ability to switch directories #100

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AlexDCraig
Copy link

@AlexDCraig AlexDCraig commented Oct 11, 2021

This allows the user to change the directory in which the tagging logic runs.

@AlexDCraig
Copy link
Author

@mathieudutour any thoughts on this small but impactful change?

Copy link
Owner

@mathieudutour mathieudutour left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! A couple of notes

try {
const originDirectory = process.cwd();
process.chdir(repoPath);
console.log('Changed directory from ' + originDirectory + ' to ' + process.cwd());
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
console.log('Changed directory from ' + originDirectory + ' to ' + process.cwd());
core.info(`Changed directory from ${originDirectory} to ${process.cwd()}`);

process.chdir(repoPath);
console.log('Changed directory from ' + originDirectory + ' to ' + process.cwd());
} catch (err) {
console.log('Error encountered when changing directory: ' + err);
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 we should fail the workflow if the chdir fails, otherwise it might lead to unexpected behaviors

@@ -52,6 +52,9 @@ inputs:
description: "Do not perform tagging, just calculate next version and changelog, then exit."
required: false
default: "false"
repo_path:
description: "Path relative to $GITHUB_WORKSPACE that contains the cloned repository."
Copy link
Owner

Choose a reason for hiding this comment

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

Could you also update the doc in the README (with some info as to when you'd want to do that)?

@suyog-fp
Copy link

suyog-fp commented Jan 25, 2022

@AlexDHoffer can you pls fix the code as per @mathieudutour's comments?
We need this feature :D

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