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

Automatic notification about config.json changes through CI #4278

Closed
6r1d opened this issue Feb 13, 2024 · 7 comments
Closed

Automatic notification about config.json changes through CI #4278

6r1d opened this issue Feb 13, 2024 · 7 comments
Assignees
Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST

Comments

@6r1d
Copy link
Contributor

6r1d commented Feb 13, 2024

As we discussed with Vasily, it'll be much more convenient if he's notified about any possible configuration change.
Ultimately, we concluded that it's best to have a CI utility that doesn't use any outside software and notifies him about the changes.
It's not a priority and probably will be added in March 2024.

@s8sato
Copy link
Contributor

s8sato commented Feb 13, 2024

#4276 would be one of the possible solutions

@s8sato s8sato mentioned this issue Feb 19, 2024
5 tasks
@s8sato
Copy link
Contributor

s8sato commented Feb 22, 2024

#4309 (comment)

This is a new feature introduced by #4276 to automatically label and notify someone of specific code changes. Currently config-changes reacts to any change under configs/, which may be an inappropriate pattern. I guess configs/*.template.* would be correct one at the current directory structure. Since things may have changed after #4239 I'd hear your opinion @0x009922

@BAStos525
Copy link
Member

#4309 (comment)
This is a new feature introduced by #4276 to automatically label and notify someone of specific code changes. Currently config-changes reactions to any change under configs/, which may be an inappropriate pattern. I guess configs/*.template.* would be correct one at the current directory structure. Since things may have changed after #4239 I'd hear your opinion @0x009922

Yeah, lets's update notifications, please. There should not be a false-positive notifications for DevOps.

@0x009922
Copy link
Contributor

Yes, I would say that configs/{client|peer}.template.toml is an exhaustive coverage to see if there are user-facing changes in the config.

However, these files aren't covered yet with tests to ensure that the config implementation actually reflects them. There is an issue for it: #4288

@s8sato
Copy link
Contributor

s8sato commented Feb 26, 2024

#4278 (comment)

Thank you @0x009922 I think #4316 can be applied on the assumption that the templates will reflect actual configurations, which I noted in #4316

@BAStos525
Copy link
Member

Is it still relevant?

@s8sato
Copy link
Contributor

s8sato commented Mar 19, 2024

Closed as the rest of the issue will be left to #4288

@s8sato s8sato closed this as completed Mar 19, 2024
@nxsaken nxsaken added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

No branches or pull requests

5 participants