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!: wip first naive json db semantic-conv #63

Merged
merged 55 commits into from
Apr 14, 2024

Conversation

ronenkapelian
Copy link
Contributor

MapColonies implementaion for semantic convention of project attributes

Question Answer
Bug fix
New feature
Breaking change
Deprecations
Documentation
Tests added
Chore
  • first imlementation of static db (json) based.

  • Raster domain oriented

@ronenkapelian ronenkapelian self-assigned this Mar 28, 2024
@CptSchnitz CptSchnitz changed the title feat: wip first naive json db semantic-conv feat!: wip first naive json db semantic-conv Apr 8, 2024
Copy link
Collaborator

@CptSchnitz CptSchnitz left a comment

Choose a reason for hiding this comment

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

GJ!
Please go over the code files and improve the spacing and grouping of related code parts.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
package.json Outdated
@@ -11,11 +21,14 @@
"lint": "eslint .",
"lint:fix": "eslint --fix .",
"release": "standard-version",
"prebuild": "npm run clean",
"prebuild": "npm run clean && npm run validate && npm run generateTypes && npm run generateAttributes",
Copy link
Collaborator

Choose a reason for hiding this comment

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

generate types should not run in prebuild.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed.

package.json Outdated
Comment on lines 29 to 30
"generateAttributes": "node --import=tsimp/import scripts/build-semantic-conventions.mts",
"generateTypes": "json2ts schemas/attribute.schema.json > scripts/autogen.mts",
Copy link
Collaborator

Choose a reason for hiding this comment

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

lets change is so the format of the name is generate:types like with format and lint

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

Choose a reason for hiding this comment

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

Add descriptions to the various fields

Comment on lines 8 to 89
## example
Below are short examples for schema definition and domain.json files.

### Some json-schema definition
```json
{
"$schema": "http://json-schema.org/draft-07/schema",
"type": "object",
"additionalProperties": false,
"title": "cat family hierarchy schema",
"required": ["clan", "content"],
"properties": {
"clan": {
"type": "string"
},
"content": {
"$ref": "#/definitions/recursive"
}
},
"definitions": {
"recursive": {
"type": "object",
"additionalProperties": false,
"title": "clan tree",
"required": ["propertyName", "description"],
"properties": {
"propertyName": {
"type": "string"
},
"kind": {
"type": "string"
},
"description": {
"type": "string"
},
"deprecated": {
"type": "boolean"
},
"subAttributes": {
"type": "object",
"additionalProperties": {
"$ref": "#/definitions/recursive"
}
}
}
}
}
}
```

### Convert json-schema definition into typescripts
running CLI (json2ts)

```bash
npx json2ts schemas/cats.schema.json > scripts/cats.mts
```

Will expect ts autogenerated module with types

```typescript
/* eslint-disable */
/**
* This file was automatically generated by json-schema-to-typescript.
* DO NOT MODIFY IT BY HAND. Instead, modify the source JSONSchema file,
* and run json-schema-to-typescript to regenerate this file.
*/

export interface ConventionSchema {
domain?: string;
content: AttributeDescription;
}
export interface AttributeDescription {
propertyName: string;
kind?: string;
description: string;
deprecated?: boolean;
subAttributes?: {
[k: string]: AttributeDescription;
};
}
```

Copy link
Collaborator

Choose a reason for hiding this comment

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

As we talked, this is not needed, only short section on when and how to generate types

src/semanticConventions/README.md Outdated Show resolved Hide resolved
src/semanticConventions/README.md Outdated Show resolved Hide resolved
src/semanticConventions/README.md Outdated Show resolved Hide resolved
src/semanticConventions/README.md Show resolved Hide resolved
@shimoncohen
Copy link
Contributor

If this a WIP, then please don't add the feat! commitlint type.
@CptSchnitz What do you think?

@CptSchnitz
Copy link
Collaborator

If this a WIP, then please don't add the feat! commitlint type.
@CptSchnitz What do you think?

I added this as the ts config changes breaks existing code.
and the output is not wip.

@shimoncohen
Copy link
Contributor

If this a WIP, then please don't add the feat! commitlint type.
@CptSchnitz What do you think?

I added this as the ts config changes breaks existing code. and the output is not wip.

Ok, NP.

src/semanticConventions/README.md Outdated Show resolved Hide resolved
src/semanticConventions/README.md Outdated Show resolved Hide resolved
src/semanticConventions/README.md Outdated Show resolved Hide resolved
src/semanticConventions/README.md Outdated Show resolved Hide resolved
src/semanticConventions/README.md Outdated Show resolved Hide resolved
src/semanticConventions/README.md Outdated Show resolved Hide resolved
src/semanticConventions/README.md Outdated Show resolved Hide resolved
src/semanticConventions/README.md Outdated Show resolved Hide resolved
src/semanticConventions/README.md Outdated Show resolved Hide resolved
src/semanticConventions/README.md Outdated Show resolved Hide resolved
@CptSchnitz CptSchnitz merged commit 646f441 into master Apr 14, 2024
5 checks passed
@CptSchnitz CptSchnitz deleted the semanticsAttributesInfra branch April 14, 2024 07:48
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