Mage conversion #2652
Mage conversion #2652
Conversation
|
This looks really really good! I will make sure to review it over the coming days. Maybe we can comment out the make file equivalent if you have implemented it / is doesn't need to exist anymore? Just for tracking purposes. |
|
@Sharpz7 thanks, I have nearly covered every command, 1-2 commands are remaining, will add them soon |
+1 for this. I'd love to switch over our github actions to use these mage targets so we can phase out the make file. @nitishchauhan0022 Are you up for that? |
|
@kannon92 yep! i can take that. |
Great! Looking forward to reviewing. Note that by using the mage targets in the CI you can have higher confidence that they have been implemented correctly :)) |
|
@Sharpz7 definitely, will work on this this weekend.
|
| @@ -33,7 +33,7 @@ jobs: | |||
| - name: Save docker images to artifact | |||
| run: | | |||
| mkdir -p docker-images | |||
| docker save armada | gzip > docker-images/armada.tar.gz | |||
| docker save armada-server | gzip > docker-images/armada-server.tar.gz | |||
There was a problem hiding this comment.
How is this related? @richscott or @dejanzele would this break anything?
There was a problem hiding this comment.
we can rename the gorelease armada-server output to armada if needed. But as that was added already, i made the change here.
|
I find the rebase notifications to be hard to find so just want you to let you know that you should rebase! |
nitishchauhan0022
force-pushed
the
mage-conversion
branch
from
b7c614d
to
ab8f360
Compare
nitishchauhan0022
force-pushed
the
mage-conversion
branch
from
8a6ba40
to
7e0fba0
Compare
|
We have some questions for @ClifHouck on that PR so its not quite ready to merge yet. |
|
@nitishchauhan0022 we also have more CI tests, but we are only able to enable them on this PR for you if you resolve the merge conflicts. Thanks :)) |
I decided to merge @ClifHouck PR actually so you should get his changes when you rebase. |
nitishchauhan0022
force-pushed
the
mage-conversion
branch
from
7e0fba0
to
2b774d0
Compare
|
That is CI enabled for you. If this passes we should be good to go with all other comments addressed. |
nitishchauhan0022
force-pushed
the
mage-conversion
branch
2 times, most recently
from
78b1613
to
5bf0b88
Compare
Signed-off-by: ntishchauhan0022 <nitishchauhan0022@gmail.com>
nitishchauhan0022
force-pushed
the
mage-conversion
branch
from
5bf0b88
to
73f3fd9
Compare
Signed-off-by: ntishchauhan0022 <nitishchauhan0022@gmail.com>
nitishchauhan0022
force-pushed
the
mage-conversion
branch
from
5e75dca
to
28ddbf8
Compare
Signed-off-by: ntishchauhan0022 <nitishchauhan0022@gmail.com>
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## master #2652 +/- ##
=======================================
Coverage 16.21% 16.21%
=======================================
Files 160 160
Lines 12559 12559
Branches 487 487
=======================================
Hits 2037 2037
Misses 10353 10353
Partials 169 169
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Please fix the merge conflicts. After that I think this is mergable. |
Fixes #2511
Special notes for your reviewer:
┆Issue is synchronized with this Jira Task by Unito