test(weaver/indy-plenum): fix dockerfile - use pip3 install instead of setup.py

[rudranshsharma123]

Fixes issue #2500
Changed the line 24 of the dockerfile, to remove the use of the erroneous setup.py to replace it with a full line of all the dependencies modeled from the same file.

@VRamakrishna Kindly check

[ghoshbishakh]

The issue seems to be in upstream:

https://github.com/hyperledger/indy-plenum/blob/main/setup.py#L38

Here the --install-options is used which is now deprecated https://pip.pypa.io/en/stable/news/#deprecations-and-removals

Please create an issue in the plenum repo: https://github.com/hyperledger/indy-plenum/issues

[sandeepnRES]

@rudranshsharma123 also use the same message in PR title as commit message, we actually keep PR title same as the commit message. also It'd be better to be specifically saying fix(iin), instead of fix(dockerfile), since there are lot of dockerfiles.

[sandeepnRES]

@rudranshsharma123 Try to have the commit messages (and PR Title) more informative. ("change the dockerfile to reflect changes" doesn't tell anything about the change).

For now I've changed PR title to be more expressive in terms of what is this commit doing. You can use the same in commit message.

[petermetz]

@rudranshsharma123 Thank you for the fix! I agree with @sandeepnRES 100%, it looks good but please make sure that the information you put in the PR description is also present in the git commit message.
They seem like they are the same thing but unfortunately they are not - one is stored in a private database of GitHub the other is the actual open source git commit message that the community owns.

If you need help with amending commit messages or anything git related in general then I'm happy to help with either advice or just doing it for you, just let me know!

[rudranshsharma123]

Hi @petermetz I have amended the commit message to add the description of the PR in it as well. I have made it so that the first line still remains the summary of the fix and then a line break and then the description of the PR. Kindly let me know if it's ok or if I would need to make further changes. Thank you!

[petermetz]

@rudranshsharma123 Looks much better now thank you! Now it's just 2 last things:

1. The commit linter is failing because you have one very long line instead of a tall column. Please see the linter errors pasted at the end of my comment.
2. The subject line of the commit is still pretty ambiguous because a lot of the commits that we do that involve docker files to the exact thing of changing how dependencies are installed (version upgrades, adding new deps, removing old ones, etc.). So what I recommend as a commit subject line is something that gets to the details a little more which allows someone just by reading the subject line to get a rough idea of what you actually changed in the file without having to look at the body of the commit message (this makes it so much easier to parse through the change log in the release notes because there only the subject lines are shown not the entire body of the commit message).
   My recommendation would be something like:
   test(weaver/indy-plenum): fix dockerfile - use pip3 install instead of setup.py
   The other reason that this is better is because then this won't actually make it to the release notes because it's a test code change which has no actual impact on anything for someone who has the code deployed and running in production. E.g. when they are reading the release notes and trying to decide whether to upgrade to the new release version or not - reading about our test docker container/code fixes are just noise to them rather than a signal)
   More relevant context: Using the fix qualifier in the commit subject implies that we've fixed a bug that was affecting code running in production

---

Error: You have commit messages with errors

⧗   input: fix(iin): changed the way to install dependencies in dockerfile

Changed the line 24 of the dockerfile, to remove the use of the erroneous setup.py to replace it with a full line of all the dependencies modeled from the same file.

Signed-off-by: Rudransh Sharma <rudranshsharma@Rudranshs-MacBook-Pro.local>
✖   body's lines must not be longer than 100 characters [body-max-line-length]

✖   found 1 problems, 0 warnings
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

[rudranshsharma123]

Hi @petermetz, I think I have tried to fix the issues with the commit message as you requested. I have made it so that the one big line is split into four lines conveying the same message and have changed the heading of the commit message to reflect test instead of fix. I also added the changes made to the upstream which added another commit.

Kindly let me know if further changes are needed.
Thank you!

[petermetz]

Hi @petermetz, I think I have tried to fix the issues with the commit message as you requested. I have made it so that the one big line is split into four lines conveying the same message and have changed the heading of the commit message to reflect test instead of fix. I also added the changes made to the upstream which added another commit.

Kindly let me know if further changes are needed. Thank you!

@rudranshsharma123 Thank you very much, we are looking much better now! Just a few more things:

1. Once you've made the requested changes you can do a re-request review (there's a button for it in the top right of the PR page, next to my name in the Reviewers section). In this case it did not matter because I caught your message anyway but sometimes I'll only have time to check pull requests where my review was requested and right now this wouldn't be caught in that filter. This is not a problem with the PR I just wanted to explain it for future reference.
2. There has to be only one commit so if you can do an interactive rebase and make the current two commits into just one then we'll be almost ready. The reason a new commit got added is because you've merged main into your branch instead of rebasing your branch onto main. If you need help with this just let me know.
3. The PR description and the commit message should be in sync. You declared Fixes issue https://github.com/hyperledger/cacti/issues/2500 in the PR description but not in the commit message so I would also put it in the commit message so that future generations can find this information even if we end up migrating away from GitHub (no such plans but it's just good practice to keep as much information in vanilla git as possible vs. proprietary databases that we don't have ownership of as a community)

So it looks mostly good now but if you could fix 2 and 3 then we'd be ready to merge!

[petermetz]

LGTM