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

[vscode][5/n] Setup Env Variables: Read the text from the env template and append to existing file #1285

Merged
merged 2 commits into from
Feb 22, 2024

Conversation

rossdanlm
Copy link
Contributor

@rossdanlm rossdanlm commented Feb 22, 2024

[vscode][5/n] Setup Env Variables: Read the text from the env template and append to existing file

TSIA, and making the function async now that we call an await statement

Test Plan

06dbbee1-9e3c-441f-8a5c-2391ba661481.mp4

Stack created with Sapling. Best reviewed with ReviewStack.

Rossdan Craig [email protected] added 2 commits February 21, 2024 19:58
This PR I had some issues with reading the env template, so for now I'm splitting up this logic and will follow up next PR

## Test Plan

https://github.com/lastmile-ai/aiconfig/assets/151060367/6aad117f-0c55-4549-8f46-1b438c6ebf4b
// });
const helperText = (
await vscode.workspace.fs.readFile(envTemplatePath)
).toString();
Copy link
Member

Choose a reason for hiding this comment

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

nice! using const instead of let


// TODO: Check if we already appended the template text to existing .env
// file before. If we did, don't do it again
fs.appendFile(envPath, helperText, function (err) {
fs.appendFile(envPath, "\n\n" + helperText, function (err) {
Copy link
Member

@Ankush-lastmile Ankush-lastmile Feb 22, 2024

Choose a reason for hiding this comment

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

nit: define \n\n as a constant


// TODO: Check if we already appended the template text to existing .env
// file before. If we did, don't do it again
fs.appendFile(envPath, "\n\n" + helperText, function (err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine for now, but if a file already exists perhaps we should ask the user to specify a different file or ask them if they want to overwrite or append. Can be a future improvement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to push back saying this is P1 and focus more on content for tonight. If I have time before midnight I'll do it

Copy link
Member

@Ankush-lastmile Ankush-lastmile left a comment

Choose a reason for hiding this comment

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

lgtm, approving to unblock

rossdanlm added a commit that referenced this pull request Feb 22, 2024
…th (#1284)

[vscode][4/n] Setup Env Variables: Add helper text to existing env path

This PR I had some issues with reading the env template, so for now I'm
splitting up this logic and will follow up next PR

## Test Plan


https://github.com/lastmile-ai/aiconfig/assets/151060367/6aad117f-0c55-4549-8f46-1b438c6ebf4b

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/1284).
* #1285
* __->__ #1284
* #1283
* #1280
@rossdanlm rossdanlm merged commit ec9cbb8 into main Feb 22, 2024
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