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

feat: add index resolution for modules, resources and segments #590

Merged

Conversation

basmasking
Copy link
Member

@basmasking basmasking commented Dec 27, 2024

Fixes #572

Changes proposed in this pull request:

  • resolve index.js in case of directory reference

There are no tests yet, these will be added in #583. This feature can be tested using Comify, branch #361

@MaskingTechnology/jitar

@basmasking basmasking linked an issue Dec 27, 2024 that may be closed by this pull request
petermasking
petermasking previously approved these changes Dec 30, 2024
{
text: 'BUILD',
items: [
{ text: 'Settings', link: '/build/settings' },
Copy link
Member

Choose a reason for hiding this comment

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

The term settings feels a bit weird in the context of providing arguments for the CLI parameters. Maybe something like arguments, parameters or flags is a better fit?

For a TypeScript project, the `source` folder should be the target folder after transpilation, so it should be `./dist` instead of `./src`. The `target` folder can be the same as the `source` folder in this case, but it can also be a different folder.
:::

::: tip NOTE
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
::: tip NOTE
::: warning IMPORTANT

The build process deletes the files in the `target` folder during the build process. Make sure that it doesn't point to the `src` folder.
:::

::: tip NOTE
Copy link
Member

Choose a reason for hiding this comment

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

This note contains general configuration information, so doesn't have to be a not in my opinion. Should we move the text up and place it above the first note? This also avoids a stack of notes.

@@ -19,12 +19,12 @@ In Jitar's [segmentation model](/deploy/segmentation), each segment is isolated

### Resource files

Jitar will search for resource definitions files in the project directory. The files are named `*.resources.json`. Each entry defines the entry point of the `module` that should be used as a resource.
Jitar will search for resource definitions files in the `resources` directory. The files are named `*.json`. Each entry defines the entry point of the `module` that should be used as a resource.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Jitar will search for resource definitions files in the `resources` directory. The files are named `*.json`. Each entry defines the entry point of the `module` that should be used as a resource.
Jitar reads resource definitions files from the `resources` directory. The files are in JSON format. Each entry defines the entry point of the `module` that should be used as a resource.

Comment on lines +104 to +107
└─ integrations
├─ database
├─ notifications
├─ ...
Copy link
Member

Choose a reason for hiding this comment

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

This is a part of our future-proof vision. Should we reflect it in the Jitar docs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Created the following Issue so we can keep track of these items.


console.log(person);
console.log(copy);
Copy link
Member

Choose a reason for hiding this comment

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

How does logging the copy proves that the original hasn't been modified?

packages/sourcing/src/LocalFileSystem.ts Show resolved Hide resolved
Copy link

sonarqubecloud bot commented Jan 3, 2025

@petermasking petermasking merged commit 8ecd087 into main Jan 3, 2025
22 checks passed
@petermasking petermasking deleted the 572-add-index-resolution-for-configuration-files-v2 branch January 3, 2025 13:13
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.

Add index resolution for configuration files
2 participants