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

1475 fix home envvar #1621

Merged
merged 6 commits into from
Feb 9, 2024
Merged

Conversation

richardm90
Copy link
Contributor

Changes

This PR relates to issue #1475.

I have modified CompileTools.ts so that it no longer sets the HOME variable, which goes on to set the PASE HOME environment variable. I have also added a new variable called WORKDIR, as a replacement for HOME.

In doing so the HOME variable is no longer available for use in actions.

IMPORTANT Is there a way to advise users that their actions will need updating?

I have also added another new variable called FILEDIR that gives you the remote directory that the file exists. This is extremely useful when you have multiple Makefiles in your project. You can have an action command such as cd &FILEDIR && /QOpenSys/pkgs/bin/gmake.

I have also updated the varinfo.ts and appropriate locale files with the action variable description changes. I used Google Translate on the French and Danish versions so if someone could check these that would be useful.

Checklist

  • have tested my change
  • updated relevant documentation
  • Remove any/all console.logs I added
  • eslint is not complaining
  • have added myself to the contributors' list in CONTRIBUTING.md
  • for feature PRs: PR only includes one feature enhancement.

src/api/CompileTools.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@worksofliam worksofliam left a comment

Choose a reason for hiding this comment

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

Let's keep &FILEDIR for sure, but let's also keep both &WORKDIR plus &HOME for compatibility reasons.

When you have added &HOME back in (you don't need to add it back to the variable list UI) then I will review again.

@richardm90
Copy link
Contributor Author

Hey @worksofliam was my update OK?

@worksofliam worksofliam added this to the 2.6.0 milestone Dec 11, 2023
@worksofliam
Copy link
Contributor

@richardm90 Sorry for leaving this for so long. We had a big release and we also had to solve a big bug - I've also added &WORKDIR in #1703 - sorry for the duplicate efforts, but that change was larger and solve a very big bad bug.

I think this PR should now be used to add &FILEDIR as that is incredibly useful as you described. You're welcome to update this PR or create a new one. Sorry for the trouble.

@worksofliam worksofliam removed this from the 2.6.0 milestone Dec 11, 2023
@richardm90
Copy link
Contributor Author

@worksofliam , took me a little while as I was getting a problem when testing which must have been sorted in a recent commit as pulling down the latest has resolved my problem. I've updated the PR as suggested.

@worksofliam worksofliam self-assigned this Jan 7, 2024
@worksofliam
Copy link
Contributor

@richardm90 I am sorry - every time I come around to look at this there are new conflicts. Any chance you can fix those and I can get another review in?

@richardm90
Copy link
Contributor Author

@richardm90 , no problem, I'll get to it this week along with the comments you left on the other pr

Copy link
Contributor Author

@richardm90 richardm90 left a comment

Choose a reason for hiding this comment

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

@worksofliam I've applied the latest changes

src/api/CompileTools.ts Outdated Show resolved Hide resolved
@worksofliam worksofliam added this to the 2.7.0 milestone Feb 7, 2024
Copy link
Contributor

@worksofliam worksofliam left a comment

Choose a reason for hiding this comment

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

Looks good to me!

If you want to update this page for local development to include these changes, you're welcome: https://github.com/codefori/docs/blob/main/docs/pages/developing/local/actions.md

@worksofliam worksofliam merged commit 187c7da into codefori:master Feb 9, 2024
1 check passed
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.

$HOME environment variable being set to the workspace deploy directory
2 participants