fix: update DOCKER CMD on docs/Makefile

[viktoriussuwandi]

Fixes #1744

Hi agardnerIT,
just trying to fix issue on file lifecycle-toolkit/docs/Makefile, please check it out

[netlify]

✅ Deploy Preview for keptn-lifecycle-toolkit ready!

Name	Link
🔨 Latest commit	31877ea
🔍 Latest deploy log	https://app.netlify.com/sites/keptn-lifecycle-toolkit/deploys/64c23a5f40917c0008e51b18
😎 Deploy Preview	https://deploy-preview-1745--keptn-lifecycle-toolkit.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[agardnerIT]

LGTM! Thanks! Closes #1744

[agardnerIT]

Unfortunately it seems I was wrong with my suggested fix. The -it flag works perfectly for us humans (as we do have an interactive terminal) but GitHub Actions apparently doesn't like it 😞

https://github.com/keptn/lifecycle-toolkit/actions/runs/5606326111/jobs/10257542019?pr=1745

[viktoriussuwandi]

Unfortunately it seems I was wrong with my suggested fix. The -it flag works perfectly for us humans (as we do have an interactive terminal) but GitHub Actions apparently doesn't like it 😞

https://github.com/keptn/lifecycle-toolkit/actions/runs/5606326111/jobs/10257542019?pr=1745

Hi agardnerIT,
Would you please let me know anything else needed to do to merge this PR ?
Or should it close without merge ?

[agardnerIT]

Due to the issue identified above and the fact that making the change breaks other compatibility (ie. when make server runs in a PR). I think the best way to achieve this would be to add an entirely new task in the Makefile. You can copy and paste the make server command and for this new one, just add the -it flag.

In other words, make server remains as it always has, but this PR introduces a new task which is identical to make server except it includes the -I flag. The intention of this new target would be for contributors (eg. humans) to run this task. The CI processes will continue to run make server as they always have.

When this new task is introduced, the docs/README.md should be updated to update the command.

lifecycle-toolkit/docs/README.md

Line 14 in 92730ad

make server

Perhaps also, add a note to docs/README.md explaining the different between make server and the new command.

Sorry for the extra work, but I just don't see another way to achieve this without breaking other things.

[viktoriussuwandi]

Thanks for your explanation @agardnerIT,

Actually I don't know how to apply your idea, but this is what I have tried to do :

1. Restore docs/Makefile as before with just -i flag on line 7

2. Create new file docs/Makeserver with exactly same code as docs/Makefile, with -it flag on line 7

Please let me know anything need else to do or improve. Appreciate your guidance !

[aepfli]

i would not suggest to duplicate this file. It creates confusion, and does not make it logical on why to use it or not.

eg. this can be determined within the makefile (eg. https://stackoverflow.com/a/4251643/3708208) and we can set a variable based on this definition. Such an approach would make it way clearer to the users, und why there is a difference, and in the end, the end user does not want to ensure a proper file or not.

[mowies]

what @aepfli said :D

[mowies]

@viktoriussuwandi pls check the comment here: #1745 (comment)

[viktoriussuwandi]

Hi @mowies
I have tried to update as mentioned by the reference, hopefully it will run as expected.

@aepfli Thanks for your suggestion! Also need your feedback on this

[mowies]

one small change, other than that, LGTM

[viktoriussuwandi]

no problem @mowies, Thanks for your suggestion!

[mowies]

LGTM

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[agardnerIT]

That's an awesome solution. Definitely learned something here!

[viktoriussuwandi]

Thanks for this opportunity @ALL

[aepfli]

Nice Solution! well done!