feat/refactor: add automock lib and replace FakeLoreAPIClient with automocked BlockingLoreAPIClient

[lorenzoberts]

This PR does two things

1. adds the "automock" library to Cargo.toml, which is used to automatically mock traits and structs, avoiding the need for manual implementation of each trait function just for testing.
2. replaces the FakeLoreAPIClient used in src/lore_session/tests.rs with an "automock" for BlockingLoreAPIClient. This way, each test can have an explicit expectation for each of the called functions, facilitating test implementation and improving test quality overall.

Closes: #34

[lorenzoberts]

@davidbtadokoro I don't know why my PRs (#44 included) have this warning "2 workflows awaiting approval". This is preventing the CI to run and I am wondering if it has to do with those workflows I created for linting and formating in the past. See if you can reject them, maybe then the rest of the workflows will run for my user

[davidbtadokoro]

Hey @lorenzoberts, thanks a lot for this PR. It looks really promising. Like the other one you've opened, I will look at it ASAP.

@davidbtadokoro I don't know why my PRs (#44 included) have this warning "2 workflows awaiting approval". This is preventing the CI to run and I am wondering if it has to do with those workflows I created for linting and formating in the past. See if you can reject them, maybe then the rest of the workflows will run for my user

True! When I review your changes I will do this first thing. I've noticed those, but didn't figured out the reasoning behind it. It makes sense your theory about the matter, but I will tackle it with more assertive ASAP.

[davidbtadokoro]

@me:

True! When I review your changes I will do this first thing. I've noticed those, but didn't figured out the reasoning behind it. It makes sense your theory about the matter, but I will tackle it with more assertive ASAP.

Just to update you, I quickly dug through reasonings on why I need to approve the workflows from you and came up with this:

GitHub policy on workflow runs from public forks

I don't know if you can see it but besides the "approve and run" button for the blocked workflows there is a link to a GitHub doc page (print below).

I followed the link and end up with the following information:

"[...] workflows can be an annoyance for maintainers if they are modified for abusive purposes. To help prevent this, workflows on pull requests to public repositories from some outside contributors will not run automatically, and might need to be approved first. By default, all first-time contributors require approval to run workflows."

I found it strange, as I remember that PR from @OJarrisonn and @th-duvanel didn't require approval for workflow runs, but then I dug deeper and found that their first PR also required approval to be run.

My take on the matter

There is a way to disable the necessity of the approval (print below).

However, I feel like this GitHub policy makes sense as it increases the security of the project a little bit while having just a small drawback. For example, after merging this or #44, this won't be an issue for you, specifically.

Opinions

Like any matter on the project, I am open to any divergent opinions, so anyone feel free to comment with your takes on the matter 😄

[davidbtadokoro]

About the actual review of the PR, I glanced over it and it feels legit! However, just like I commented on #44, I wish to review it more thoroughly before merging.

In any case, marvelous work! By this point, you know I am picky about commit messages, coding style, and other things that don't relate directly to the code and functionality. Know that you've nailed on those points!

P.S.: I actually didn't know we could have a commit prefix feat/refactor which makes a lot of sense! Recently, I pushed a commit that I felt fit on both categories, so it is nice to know 🤣

[lorenzoberts]

Like any matter on the project, I am open to any divergent opinions, so anyone feel free to comment with your takes on the matter

You're totally right, I think it is a good practice since anyone would be able to spam workflows, run malicious code or any possible troll script. I had never pushed workflows to public repos so I didn't know if it was something I messed up, thanks for clarifying it!
You're totally right. I think it is a good practice since anyone could spam workflows, run malicious code, or any possible troll script. I had never pushed workflows to public repos, so I didn't know if it was something I messed up. Thanks for clarifying it!

I actually didn't know we could have a commit prefix feat/refactor

Hahaha, I mean, I'm not sure if it's something common in the community, but in my view, when a commit refactors some logic by introducing another unknown tool/lib/etc., this custom conventional commit fits perfectly. Depending on the size of the change, I would split it into two PRs: one for simply adding the lib to Cargo.toml (feat), and the other to make the change itself (refactor). But in this case, I didn’t feel the need, as I only had to change two Rust files.

[davidbtadokoro]

Hey @lorenzoberts, sorry for the huge delay. Thank you for all the work and patience. Change merged into the unstable branch 👍