Build `If-env` #790

manushak · 2024-06-07T14:08:46Z

Types of changes

Enhancement (project structure, spelling, grammar, formatting)
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

A description of the changes proposed in the Pull Request

Buld if-env CLI to allow the user to setup a local environment to run a manifest file.

jmcook1186 · 2024-06-07T14:23:21Z

@manushak still reviewing this - seems good so far but an observation is that it takes quite a while to complete and during that time the user does not know what is happening - can we add some execution logs?

manushak · 2024-06-07T14:26:24Z

@manushak still reviewing this - seems good so far but an observation is that it takes quite a while to complete and during that time the user does not know what is happening - can we add some execution logs?

sure. I'll add logs

narekhovhannisyan · 2024-06-08T05:14:46Z

src/env/env.ts

+/**
+ * Updates package.json dependencies.
+ */
+async function updatePackageJsonDependencies(


why not arrow function?

narekhovhannisyan · 2024-06-08T05:15:46Z

src/env/env.ts

@@ -0,0 +1,211 @@
+#!/usr/bin/env node
+/* eslint-disable no-process-exit */
+import * as fs from 'fs/promises';


please move the file to src folder. We don't have specific folders for each script

in this case, where the if-env template manifest be placed?
@jmcook1186 should it be moved to the ./manifests/examples folder with the name if-env-template.yml?

no, it should just be saved to the top level src directory. if-env is not supposed to be run inside the if source repository

narekhovhannisyan · 2024-06-08T07:55:09Z

src/env/env.ts

+
+const FOLDER_NAME = 'if-environment';
+
+type EnvironmentOptions = {


type can be moved

narekhovhannisyan · 2024-06-08T07:55:50Z

src/env/env.ts

+): {[path: string]: string} => {
+  const paths = Object.keys(plugins).map(plugin => plugins[plugin].path);
+  const uniquePaths = [...new Set(paths)].filter(path => path !== 'builtin');
+  const pathsWithVersion: {[path: string]: string} = {};


extract as a type

narekhovhannisyan · 2024-06-08T07:57:55Z

src/env/env.ts

+/**
+ * Installs packages from the specified dependencies in the specified folder.
+ */
+async function installDependencies(


this kind of helpers can be moved to utils

narekhovhannisyan · 2024-06-24T09:45:36Z

@manushak why in one case we are listing them and in another don't?

manushak · 2024-06-24T10:34:41Z

@narekhovhannisyan, we decided that when running with the --cwd flag, it should add dependencies to the existing package.json. For example, if another manifest file already exists in the root directory, running the command should append dependencies so that both manifests can be run properly.

cc: @jmcook1186

jmcook1186 · 2024-06-24T15:11:02Z

Let's bring this to a close asap please. @manushak please address the outstanding comments as your priority task on IF. @narekhovhannisyan please approve unless there are changes that negatively affect the behaviour - no blocking on cosmetic issues that can be tidied up later.

@zanete

manushak · 2024-06-24T15:58:03Z

@jmcook1186, @narekhovhannisyan, I've pushed the latest changes. Some functions don't have unit tests yet, but I'll add them later. I couldn't add the custom Error from if-core for throw new Error(FAILURE_MESSAGE_DEPENDENCIES);. There is a problem with husky; I couldn't link if-core with if.

narekhovhannisyan · 2024-06-24T17:24:12Z

@jmcook1186 all my feedback is about the architecture and consequential poor unit test coverage (huge amount of functions were in env.ts which is not covered with unit tests), nothing cosmetic there.

manushak · 2024-06-25T13:02:00Z

@manushak Noticed such an issue. When running from outside of IF ,with prefix and cwd, unnecessary dependencies are added into the generated package.json file. Expecting to NOT see the framework dependencies there. Please check.

Used command: npm run --prefix=./if if-env -- -m manifests/testing/testing.yaml --cwd

@MariamKhalatova, @narekhovhannisyan sorry, I got it wrong from the screenshot. I thought the selected dependencies were missing and you wanted them to persist. Thanks for noticing. It's fixed now.

package.json

Signed-off-by: Narek Hovhannisyan <[email protected]>

manushak added 10 commits June 7, 2024 17:47

feat(util): add function to check if the file exists in the given path

1adbf1d

feat(config): add 'MANIFEST_NOT_FOUND' string

152cfe3

feat(types): adds IFEnvArgs interface

5fd1e79

feat(util): add if-env releated functions

33ef812

test(util): add args test

6986792

feat(config): add IF_ENV config

13a1ab2

feat(src): add if-env template manifest file

9d0d73c

feat(src): add if-env functionality

f9d4de7

feat(package): add if-env into bin and scripts

4139704

Merge branch 'main' into if-env

1f6b3be

manushak requested review from narekhovhannisyan and jmcook1186 June 7, 2024 14:08

manushak self-assigned this Jun 7, 2024

manushak linked an issue Jun 7, 2024 that may be closed by this pull request

Build if-env #636

Closed

6 tasks

narekhovhannisyan reviewed Jun 8, 2024

View reviewed changes

src/env/env.ts Outdated

const FOLDER_NAME = 'if-environment';

type EnvironmentOptions = {

Copy link

Member

narekhovhannisyan Jun 8, 2024

Choose a reason for hiding this comment

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

type can be moved

narekhovhannisyan reviewed Jun 8, 2024

View reviewed changes

manushak added 9 commits June 10, 2024 19:05

fix(src): move template manifest to the top level of the scr folder

10cf52a

feat(config): add log strings

caf68b1

feat(types): add env types

a28f05b

feat(util): move some env functions into helpers.ts

ec3ce36

fix(src): move env to the src folder

1f0700e

fix(package): fix env file path

4583da3

test(mocks): add fs.stat mock function

9dec82b

test(util): add test for npm manipulation functions in helpers.ts

ec43b2d

test(util): fix commit lint error

91c1bed

manushak added 4 commits June 24, 2024 19:28

fix(util): add some function into helpers.ts from env.ts

67e1595

fix(util): remove unnecessary logs

b3e49bb

test(util): add test for helpers function

3bfa285

fix(src): move some funcions into helpers

276589f

manushak added 8 commits June 25, 2024 15:48

fix(src): fix merge conflicts with main

fc8e42d

fix(util): change CliInputError to ParseCliParamsError error

13a7019

test(util): fix npm test

66a1c4c

test(mocks): add mocks for fs.writeFile

0e789b1

test(util): add missied tests

accf32f

fix(util): add MissingPluginDependenciesError custom error class

d46d71c

feat(package): update if-core version

04e514a

fix(util): fix dependencies append logic

ad2f87d

narekhovhannisyan added 3 commits June 26, 2024 10:01

fix(util): typo in npm

621a311

test(util): add cases for initializeAndInstallLibs

136f062

test(util): put back original process exit

7a466cc

narekhovhannisyan reviewed Jun 26, 2024

View reviewed changes

package.json Outdated Show resolved Hide resolved

fix(src): add newline to package.json

ed31d52

Signed-off-by: Narek Hovhannisyan <[email protected]>

narekhovhannisyan approved these changes Jun 26, 2024

View reviewed changes

MariamKhalatova approved these changes Jun 26, 2024

View reviewed changes

MariamKhalatova merged commit 62928e7 into main Jun 26, 2024
2 checks passed

MariamKhalatova deleted the if-env branch June 26, 2024 08:23

This was referenced Jun 28, 2024

Release v0.5.0-beta.0 #868

Merged

Release v0.5.0 #884

Merged

github-actions bot mentioned this pull request Aug 9, 2024

Release v0.6.0 #957

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build `If-env` #790

Build `If-env` #790

manushak commented Jun 7, 2024

jmcook1186 commented Jun 7, 2024

manushak commented Jun 7, 2024

narekhovhannisyan Jun 8, 2024

narekhovhannisyan Jun 8, 2024

manushak Jun 10, 2024

jmcook1186 Jun 10, 2024

narekhovhannisyan Jun 8, 2024

narekhovhannisyan Jun 8, 2024

narekhovhannisyan Jun 8, 2024

narekhovhannisyan commented Jun 24, 2024

manushak commented Jun 24, 2024

jmcook1186 commented Jun 24, 2024 •

edited

Loading

manushak commented Jun 24, 2024

narekhovhannisyan commented Jun 24, 2024

manushak commented Jun 25, 2024


		const FOLDER_NAME = 'if-environment';

		type EnvironmentOptions = {

Build If-env #790

Build If-env #790

Conversation

manushak commented Jun 7, 2024

Types of changes

A description of the changes proposed in the Pull Request

jmcook1186 commented Jun 7, 2024

manushak commented Jun 7, 2024

narekhovhannisyan Jun 8, 2024

Choose a reason for hiding this comment

narekhovhannisyan Jun 8, 2024

Choose a reason for hiding this comment

manushak Jun 10, 2024

Choose a reason for hiding this comment

jmcook1186 Jun 10, 2024

Choose a reason for hiding this comment

narekhovhannisyan Jun 8, 2024

Choose a reason for hiding this comment

narekhovhannisyan Jun 8, 2024

Choose a reason for hiding this comment

narekhovhannisyan Jun 8, 2024

Choose a reason for hiding this comment

narekhovhannisyan commented Jun 24, 2024

manushak commented Jun 24, 2024

jmcook1186 commented Jun 24, 2024 • edited Loading

manushak commented Jun 24, 2024

narekhovhannisyan commented Jun 24, 2024

manushak commented Jun 25, 2024

Build `If-env` #790

Build `If-env` #790

jmcook1186 commented Jun 24, 2024 •

edited

Loading