Great work, thank you! I like the decoupling of validation that allows reasonably easy testing.

One nitpick though: we're trying to use conventional commits approach. I'd imagine the commit message (and maybe a PR title to avoid confusion) along the lines of feat(config): add validation.

I have changed the title of the PR so that it is according to the conventional commits. I guess that this is sufficient to have that message displayed with the final commit in main branch.

@sergey-melnychuk @LKozlowski I have updated the PR based on the comments. Please review it again

conventional commits

Not sure that I follow. After finished PR, all commits should be squashed and there should be only one commit with name of the PR feat(config): add validation, which is according to conventional commits format. This branch would be deleted, so everything looks good to me. Please correct me if I understood something wrong

conventional commits

Not sure that I follow. After finished PR, all commits should be squashed and there should be only one commit with name of the PR feat(config): add validation, which is according to conventional commits format. This branch would be deleted, so everything looks good to me. Please correct me if I understood something wrong

The idea behind that convention is that we only want meaningful commits with a bit of metadata in commit messages in the git history. In this case, we want to avoid introducing commits like "fixed clippy" into history and leave one commit that would tell us what was changed.
You are right, and as a result of your work in this pull request you should have one commit with the message "feat(config): add validation".
Of course, in the future, there might be cases where the scope of the pull request is bigger so you'll end up with more commits, but all of them should follow that convention 😄

conventional commits

Not sure that I follow. After finished PR, all commits should be squashed and there should be only one commit with name of the PR feat(config): add validation, which is according to conventional commits format. This branch would be deleted, so everything looks good to me. Please correct me if I understood something wrong

The idea behind that convention is that we only want meaningful commits with a bit of metadata in commit messages in the git history. In this case, we want to avoid introducing commits like "fixed clippy" into history and leave one commit that would tell us what was changed. You are right, and as a result of your work in this pull request you should have one commit with the message "feat(config): add validation". Of course, in the future, there might be cases where the scope of the pull request is bigger so you'll end up with more commits, but all of them should follow that convention 😄

Understood, I completely agree with only meaningful commits in history. I will pay attention to it in future work :)

conventional commits

Not sure that I follow. After finished PR, all commits should be squashed and there should be only one commit with name of the PR feat(config): add validation, which is according to conventional commits format. This branch would be deleted, so everything looks good to me. Please correct me if I understood something wrong

The idea behind that convention is that we only want meaningful commits with a bit of metadata in commit messages in the git history. In this case, we want to avoid introducing commits like "fixed clippy" into history and leave one commit that would tell us what was changed. You are right, and as a result of your work in this pull request you should have one commit with the message "feat(config): add validation". Of course, in the future, there might be cases where the scope of the pull request is bigger so you'll end up with more commits, but all of them should follow that convention 😄

Understood, I completely agree with only meaningful commits in history. I will pay attention to it in future work :)

As long as you're working on something, you can have all sorts of commits as it sometimes might help to review PRs, but when you are about to merge it into the main branch then it is a time for a cleanup :)

We're trying to keep the history linear (basically only fast-forward merges are allowed on the main branch), thus also try avoiding merge commits. We also don't require to squash all the commits in the PR into one, even though you can do that if you want to, so there is flexibility. If you want the PR to take only single commit - please rebase & then squash all the remaining ones.

The goal is to make small "atomic" changes that are easier to review and potentially troubleshoot. We don't have a limit on number of commits. My personal preference is to keep each change small and meaningful, even keeping fmt & clippy results (such commits are easily skipped during review and don't hurt when they sit in the branch).