@dhruv.mittal, I tested this patch and it looks good to me. Setting to RTBC
@dhruv.mittal
Thanks for looking at this ticket. I think `sass` is the option we go with.
@dhruv.mittal, thank you for the patch. It looks good to me. Changing to RTBC
@anish.ir, Thank you for providing the patch. It looks good to me. Moving it to RTBC.
yas → credited baldwinlouie → .
@yas, I've updated the patch to fix the stylelint issues.
@yas, please review these patches.
baldwinlouie → created an issue.
@yas, I'll create the 7.x and 8.x branch
@sachin.addweb. Thank you for the fixes to the patch. It looks good to me now.
@sachin.addweb, Thank you for the updated patch. It fixes the "close" icon issue.
Please take a look at this comment: https://git.drupalcode.org/project/rigel/-/merge_requests/68#note_370892
Aside from that, it is looks good to me.
@sachin.addweb, Thank you for the updated patch. I made one code comment. Also, please see attached screenshot. When you added the new "X" as the close icon, there smaller one is still showing up.
Changing to needs review to trigger testing.
@sachin.addweb, Thank you for taking over this patch. There are stylelint issues reported with main.css during Unit testing. Can you please run Step 6 from the README.md, npx prettier --config .prettierrc.json --write "assets/css/main.css". That will fix all the stylelint issues.
@yas, thank you for the update on the patch! It looks good to me now.
@yas, thank you for the patch. I just have these minor comments. Aside from that, it looks good.
@yas, thank you for the patch. This looks good to me.
@yas this looks good to me. Thank you for the patch.
@yas this looks good to me. Thank you for the patch.
@yas, thank you for the patch. This looks good to me.
@yas, thank you for the patch. It looks good to me.
@yas, please review the updated patch.
@yas please review the patch.
baldwinlouie → created an issue.
@yas, thank you for the updated patch. It looks good to me.
@yas, thank you for the patch. I have one comment.
@yas Please review this patch. This takes care of a few things
1. Fixes the white background because in Drupal 10.4+, the css class is accordion and not card anymore
2. Changes all double quotes to single quote because of Drupal coding standards reported by prettier
3. Moves css attributes around reported by prettier. For example, all transition attributes need to be moved lower in a css class
4. Setup Grunt so it automatically runs a local prettier to reformat all code based on the .prettierrc.json
@yas, thank you for the patch. It looks good to me.
@yas, thank you for the patch. It looks good to me.
@yas, thank you for the patch. It looks good to me.
@yas, thank you for the patch. This looks good to me.
baldwinlouie → created an issue.
@yas, please review this patch again. I've incorporated all your feedback. I also fixed the Unit Test: https://git.drupalcode.org/project/cloud/-/jobs/1681933
@yas, Please review this patch. This adds a new field to the K8s cloud service provider form.
When cloud store entities are imported, they will use the uid from the cloud service provider entity.
baldwinlouie → created an issue.
@yas, Thanks for the patch. It looks good to me.
@yas, Thanks for the patch. It looks good to me.
@yas, please review this patch. This fixes the hard-coded "Store Content" title on Cloud Store entities.
baldwinlouie → changed the visibility of the branch 3446961-change-hard-coded-store to hidden.
baldwinlouie → created an issue.
@yas, please review this patch. This fixes the breadcrumb spacing issue.
baldwinlouie → created an issue.
@yas, please review this patch. This fix moves the Cancel button into the correct location in a modal. It turns out the following code removes the necessary css class that the Dialog JS needs to move the button into the lower right hand corner.
$form['actions']['cancel']['#attributes']['class']['0'] = '';
I also changed the color of the Cancel button. Best practice says the the cancel button should be a different color than the main "Delete" button. That way, the user can easily distinguish between the two.
What do you think about the color change.
baldwinlouie → created an issue.
@yas, thank you for the patch. This looks good to me.
@yas, thank you for the patch. This looks good to me.
@yas, thank you for the patch. This looks good to me.
@yas, thank you for the patch. It looks good to me.
@yas, I opened a hotfix branch to fix the coding issue. 3436310-coding-fix.
Please review it.
@yas, please review the hotfix. This fixes the coder issue.
@yas, please review this patch. It adds css styling to modals.
@yas, this looks good. Thanks for the patch.
baldwinlouie → created an issue.
@yas, please review this patch. I've added a GitHub token field and removed the ability to build 4.x version of Cloud Orchestrator.
baldwinlouie → created an issue.
@yas thanks for the patch. This looks good to me.
@yas thanks for the patch. This looks good to me.
@yas thanks for the patch. This looks good to me.
@yas thanks for the patch. This looks good to me.
@yas, Thank you for the patch. It looks good to me.
@yas, this looks good. Thank you for the patch.
@yas, this patch looks good to me!
@yas, Thank you for the patch. It looks good to me.
@yas, thank you for the updated patch. It looks good to me now.
@yas, I started to add comments to the code. But since my idea applies to all the changes, I will post it here.
Instead of casting the to (string) $this->t('Save'), would it be better to use the following $this->t('Save')->render()?
What do you think?
@yas, please review this patch. I've decided that we should just remove the Cloud Orchestrator tour. Since Tour module is being deprecated, to me, that means it wasn't really useful. Rather than supporting the contributed version of Tour, let's just remove our Tour module.
baldwinlouie → created an issue.
@yas, Thank you for the patch. This looks good to me.
@yas, thank you for the patch. This looks good to me.
@yas, please review these two MRs. The change `main` branch to `6.x` and `7.x` respectively.
baldwinlouie → created an issue.
@yas, thank you for the update! The patch looks good to me now!
@yas, thank you for updating the patch. I posted my second round of comments. Basically, all `getRevision()` should be changed to `loadRevision()`. Otherwise you will encounter errors such as the following screenshot
To reproduce this error.
1. Go to any AWS Launch Template and click "Edit"
2. Then click the "Revisions" tab.
With the current `getRevision()` code, the "Revisions" tab will error out.
@yas, Thank you for working on this patch.
I think the Error::logException() looks good. My concern is the updates for revision handling.
I downloaded and ran this patch to check the revision handling.
The usage of $entity_storage->getRevision($revision_id) causes a 500 error because getRevision() method doesn't exist.
I've been playing with how to use the loadRevision from RevisionableStorageInterface. I think the following modifications should work.
Consider the usage in updateCloudLaunchTemplate in Ec2BatchOperation.php. The following adjustments made it work. You can recreate this error if you try to `Refresh` AWS launch template. The code that refreshes aws launch templates interacts with Revisions.
1. Anywhere you need to load entities with revisions, add PHPDoc comment to add typehinting.
/** @var \Drupal\Core\Entity\RevisionableStorageInterface $entity_storage */
$entity_storage = $entity_type_manager->getStorage('cloud_launch_template');
2. Change $entity_storage->getRevision back to $entity_storage->loadRevision($revision_id);.
What do you think?
@yas, I've added a MR for 6.x as well
@yas, please review this patch. This does a couple of things.
1. Fixes the path for launch_git_resource_path
2. Changes launch_git_branch_or_tag_edit to 7.x
3. Changes launch_yaml_repository to use https://raw.githubusercontent.com/docomoinnovations/cloud_orchestrator/7...
instead of the non-existent url: https://raw.githubusercontent.com/ms448/test/main/ubuntu.yaml
@yas, this looks good!
baldwinlouie → created an issue.
@yas, looks good to me!
@yas, please review this patch. This changes the launch_git_repository in k8s_common.yml for BDD testing.
baldwinlouie → created an issue.
@yas, please review again.
@yas, please review this library update.
baldwinlouie → created an issue.
@yas, please review this back to dev patch
@yas, please review this patch. We need to bump drupal/cloud to 6.0.0 to prep for Cloud Orchestrtor 6.0.0 release.
baldwinlouie → created an issue.
@yas, looks good to me
@yas, Looks good to me!
@yas, I created two new MRs.
1. https://git.drupalcode.org/project/cloud/-/merge_requests/2230#note_268665 - This one is the merge into the
5.x</code branch.
2. https://git.drupalcode.org/project/cloud/-/merge_requests/2231 - This one is a hotfix for <code>6.x branch. I need to clean up aws_cloud.libraries.yml and remove the core/jquery and cloud/select2 dependency from aws_cloud_form
@yas, this patch does the following
1. jQuery from aws_cloud_form.js.
2. Change from Select2 library to TomSelect
3. Removes Select2 from cloud.libraries.yml
4. Renames aws_cloud_image_import.css to aws_cloud_tom_select.css because the CSS can be used in Launch Template and AWS Image import form
5. Fixes a bug in AWSConstraintValidator.php. The $entity->getName() throws an exception if this code is called when creating a new AWS Launch Template. This validator gets called when the AJAX on the AWS Launch Template form tries to refresh.
baldwinlouie → created an issue.