port: allergies

[glennj]

I ported these 2 exercises recently. I believe I've incorporated the review suggestions from "all-your-base".

---

Reviewer Resources:

Track Policies

[guygastineau]

It seems you have allergies unlocked by a non-core exercise. this is causing configlet to exit with error.

If you fix that, and everything looks fine we can probably do these two together, but we prefer to do one exercise per PR when porting.

[kotp]

Please do separate them into separate pull requests. Keep this one, change the title appropriately, etc.

[glennj]

OK. I'm not sure how the unlocking really works. Kind of copying from the python track.
I'll remove beer from this PR and fix the config.

[guygastineau]

It should be unlocked by a core exercise that has already introduced the student to important concepts in the side exercise.

[glennj]

That's better. I appreciate your guidance.

[guygastineau]

I just took a quick look. That is great work.

We had some exercises in the past that relied solely on return value for true or false statements, but we decided that we should have them return a string for Boolean values.

I utilize return values in my scripts/projects for clean control flow logic, but I think that returning true or false as strings is very helpful for students newer to bash.

[glennj]

I went through the track in independent mode, and I was unaware that these example solutions even existed until I checked out this repo for contributing. Do mentored students see the example script?

Also regarding "we decided ..." -- can these decisions be added to the POLICIES or CONTRIBUTING documents? Now that I read those more carefully, I realize I did not align the .meta/version files with the canonical-data.json

[guygastineau]

Good suggestion, we should provide more detailed information about test expectations.

The students don't see the example solutions, but the test written expects the solution to use return values for Boolean answers in this case.

The example solution proves the tests are passable, and it serves as a canary if test cases will break existing solutions.

[glennj]

Oh, I see what you mean, the $status as used in the tests. Are you waiting for me to change that?

[guygastineau]

Yes.

Specifically, unless the program encounters an error we don't expect it to return a non zero value for the purposes of these test files.

So try to reserve non-zero returns for error handling.

[guygastineau]

Other than that, everything looks really good on this PR, so much m ready to merge this once the tests expect strings 😀

[guygastineau]

Thank you for the excellent work.

I am going to wait to hear back on the conversation in #279 before merging this.

If we proceed with my plan for that PR, then we can merge this as is. If we end up merging that before this bats issue is resolved as a PR, then there is a small change we can make here that will keep #279 from needing to be rebased again.

Thank your for your patience, and your work. Now that you know what to expect it will be a breeze to merge your future ports once we know how we are handling skips 😀

[guygastineau]

So, I believe #279 will be merged shortly.

We have a PR to bats, but non-rolling release users won't have access to it until their distro repos are updated.

So, if you please check out the new skip line from #279, you can use sed or whatever you want to update your test file.

I will merge this after we merge #279, but you will likely need to rerun configlet, because there are changes to the template in #279.

Seriously the last two things you will need to do before we can merge this PR 😀

[guygastineau]

Actually, #279 might be tabled for a few days, so hold off on those changes.

I will merge this soon.

[kotp]

Looks good in current version. Otherwise small changes if #279 comes in.

[guygastineau]

Thank you