search

hyperledger-cacti / cacti

Hyperledger Cacti is a new approach to the blockchain interoperability problem
https://wiki.hyperledger.org/display/cactus
Apache License 2.0
344 stars 286 forks source link

ci(github): commit parity check for PRs should show what is wrong #3470

Closed petermetz closed 2 months ago

petermetz commented 3 months ago

Description

I dogfooded the PR commit message parity check and could not figure out what the problem was (e.g. what was different in my PR description vs my commit message).

The pull request where the check is failing despite my best effort to sync up the description and the commit message: https://github.com/hyperledger/cacti/pull/3456

Acceptance Criteria

  1. The custom check should show what's exactly different between the two texts that its comparing so that it's quickly obvious to the contributor what they need to change and where.
  2. The nicest would be to show a diff like this screenshot below (or something equivalent that can be rendered as text). With that said, anything that efficiently conveys the required information will do since our time and resources are limited and we cannot put images in the GitHub CI logs anyway. image
jagpreetsinghsasan commented 3 months ago

I am working on this

jagpreetsinghsasan commented 3 months ago

@petermetz I will make the checks looser while keeping the check relevant (It would be very irritating working on a task, creating a PR and then getting stuck comparing and fixing the PR and the related commit.)

petermetz commented 3 months ago

@jagpreetsinghsasan Thank you for the work on the improvements in advance! I thought about it a little more and would strongly recommend to refactor it to be less strict by only demanding a certain level of string similarity, otherwise it will probably just end up being super annoying 90% of the time for contributors in the cases when they just have some silly typo making a difference.

I found this library that looks very versatile and perfect for the task: https://www.npmjs.com/package/string-similarity-js

Specifically I would recommend the custom check to have a 90% (0.9) similarity ratio demand by default (so maybe they won't be the exact same but as long as at least they are mostly the same we can be OK with it as a best-of-both-worlds tradeoff.

The other thing: It would be very handy to be able to configure the threshold from an env variable so that later on if we find that a higher or lower threshold is better, we don't have to touch the code at all just set the env var in ci.yaml or somewhere else where the job is defined (technically still a code-change I know, but it's in the yaml not in the actual logic)

jagpreetsinghsasan commented 3 months ago

Oh wow. This looks like a perfect solution to do a looser string matching. I will incorporate the same with a variable input of similarity ratio. Thanks @petermetz for the insights

petermetz commented 3 months ago

@jagpreetsinghsasan You got it! Thank you as well!