middlewarehq / middleware

✨ Open-source DORA metrics platform for engineering teams ✨
https://middlewarehq.com
Apache License 2.0
1.05k stars 64 forks source link

fix: added code to ensure proper execution for newer versions of docker #431

Closed sidtohan closed 3 months ago

sidtohan commented 3 months ago

Linked Issue(s)

383

Acceptance Criteria fulfillment

Changes

This PR fixes the call for docker compose within the cli. Right now, docker-compose is being used, but as of the newer versions (as stated here) , the compose command has been integrated into docker desktop itself. So we should be using docker compose instead.

For the sake of backwards compatibility, some code has been included to run the old command should the newer one fails.

CLAassistant commented 3 months ago

CLA assistant check
All committers have signed the CLA.

sidtohan commented 3 months ago

@jayantbh have made the required changes. Please have a look whenever possible. Thanks.

jayantbh commented 3 months ago

Hey @sidtohan, It still doesn't look like you ran the linter correctly.

Maybe the linter isn't working correctly, or maybe there's some other issue.

Could you share how you're running the linter?

sidtohan commented 3 months ago

I have used the command: yarn lint-fix

I switched to the cli folder before running the command.

There is one thing, when I run the above command it removes an extra new line in the cli/source/utils/run-command.ts file. But since I didn't modify that file directly, I am not sure if I should keep that particular change.

If there is anything else that I might be missing please let me know.

samad-yar-khan commented 3 months ago

@sidtohan you can just remove that extra line, thankss

I have used the command: yarn lint-fix

I switched to the cli folder before running the command.

There is one thing, when I run the above command it removes an extra new line in the cli/source/utils/run-command.ts file. But since I didn't modify that file directly, I am not sure if I should keep that particular change.

If there is anything else that I might be missing please let me know.

sidtohan commented 3 months ago

I've implemented the suggested changes. If there is anything I'm missing regarding linting please let me know. Thanks for the help. @samad-yar-khan @jayantbh

jayantbh commented 3 months ago

Let's wait for a day or so (I'm out of station at the moment and want to check why the linter didn't work as expected) Apologies for the slow movement of this PR.

jayantbh commented 3 months ago

@sidtohan this is what the linter running properly looks like: https://github.com/middlewarehq/middleware/commit/e43f11f8e2c1c0b65f189f9bce0fb5f9e5a34d19

It's not your fault. The linter isn't running properly for some reason and I had to run the eslint command manually. yarn eslint --fix ./source/**/*.*

Unfortunately I can't make a commit directly to your branch, so, the above commit I made in a duplicate branch should help.

sidtohan commented 3 months ago

@jayantbh @adnanhashmi09 I've implemented the suggested changes. However, I'm facing a merge conflict. Right now I'm pushing it as it is, but if I need to resolve the merge conflict locally first please let me know.

Thanks

adnanhashmi09 commented 3 months ago

@sidtohan Please fix the merge conflict. Also please rebase your branch to the latest changes. pull latest changes in the main branch. Then checkout to your branch and rebase your branch to latest pull of main.

sidtohan commented 3 months ago

@sidtohan Please fix the merge conflict. Also please rebase your branch to the latest changes. pull latest changes in the main branch. Then checkout to your branch and rebase your branch to latest pull of main.

Alright sure, I've made the changes. Please have a look whenever possible. Thanks :)

samad-yar-khan commented 3 months ago

@sidtohan thanks for the PR. Congrats on your first contribution at Middleware :rocket: !

sidtohan commented 3 months ago

Happy to contribute :)