Skip to content

add yaml as storage for tools#19

Merged
munyanezaarmel merged 9 commits intomainfrom
ft-yaml-storage
Mar 6, 2025
Merged

add yaml as storage for tools#19
munyanezaarmel merged 9 commits intomainfrom
ft-yaml-storage

Conversation

@munyanezaarmel
Copy link
Copy Markdown
Collaborator

Brief summary of the change made

change the tools storage from json to yaml

Are there any other side effects of this change that we should be aware of?

Describe how you tested your changes?

Pull Request checklist

Please confirm you have completed any of the necessary steps below.

  • Meaningful Pull Request title and description
  • Changes tested as described above
  • Added appropriate documentation for the change.
  • Created GitHub issues for any relevant followup/future enhancements if appropriate.

@munyanezaarmel munyanezaarmel requested a review from dmohns March 3, 2025 06:25
Copy link
Copy Markdown
Member

@dmohns dmohns left a comment

Choose a reason for hiding this comment

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

Nice, this looks good.

Just one comment/suggestion: The YAML files have quite complex structure. Do you think it would make sense to add JSON schema for the parsed YAML files and then add a small CI step to check that all YAML files pass it? For example using ajv?

@munyanezaarmel
Copy link
Copy Markdown
Collaborator Author

Nice, this looks good.

Just one comment/suggestion: The YAML files have quite complex structure. Do you think it would make sense to add JSON schema for the parsed YAML files and then add a small CI step to check that all YAML files pass it? For example using ajv?

It makes sense 100%. Let me consider this. Thanks for your review @dmohns

@munyanezaarmel
Copy link
Copy Markdown
Collaborator Author

@dmohns, could you please check again

dmohns
dmohns previously approved these changes Mar 5, 2025
Copy link
Copy Markdown
Member

@dmohns dmohns left a comment

Choose a reason for hiding this comment

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

Great :shipit:

@munyanezaarmel munyanezaarmel enabled auto-merge (squash) March 6, 2025 08:24
@munyanezaarmel munyanezaarmel merged commit 105f393 into main Mar 6, 2025
6 checks passed
@munyanezaarmel munyanezaarmel deleted the ft-yaml-storage branch March 6, 2025 08:25
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.

2 participants