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 special paths to open action #122

Merged
merged 1 commit into from
Jan 17, 2022
Merged

Conversation

Chris7X
Copy link
Contributor

@Chris7X Chris7X commented Dec 17, 2021

Added management of special paths for "open" command

Added management of special paths for "open" command
@AlbertMN AlbertMN changed the title Update actionChecker.cs Add special paths to open action Jan 17, 2022
@AlbertMN AlbertMN merged commit 6d00525 into AlbertMN:master Jan 17, 2022
@AlbertMN
Copy link
Owner

AlbertMN commented Jan 17, 2022

Hi @Chris7X :) Thanks for the addition! I merged, but gonna do some changes to your code that I didn't bother requesting a revision for. But, for future reference;

  • Code style; as for any code project, it's important to maintain one code style. In the ACC project, we put curly brackets ({) on the same line as the statement, so;
if (...) {

} else {

}

Rather than

if (...)
{

}
else
{

}

Then there were a few empty spaces and tabs, and the comments were // like this, while the standard for the rest of the code is //Like this

  • The title of the Pull Request should explain what the change does (I changed the title)

These are the few things I've corrected. I can see this is your first pull request, so well done! These are minor things, and your contribution will surely help many ACC users :)

I will try and see if I can get hotfix v1.4.6 out today, that fixes a bug and adds this.

Thanks again, keep it coming if you got anything else you wish to be added 💪

AlbertMN added a commit that referenced this pull request Jan 17, 2022
…en` actions

Made a safe version of getting environment variables, as some might not exist on some computers, resulting in issues. This should be completely exception-free,
@AlbertMN
Copy link
Owner

Ended up re-writing this one quite a bit - check the referenced commit above for more info.

After further inspection of the PR while testing, the code was not quite in the right place. Moved it from being a part of the secondary parameter check, to being an independent function that is used by the open actions as well as a few other actions where paths are a part of the parameters.

Still using some of the base-code, just restructured quite a bit :)

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