Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CIF-1529 - Include CIF Add-On in IT setup for Cloud SDK #352

Merged
merged 4 commits into from
Aug 3, 2020
Merged

Conversation

cjelger
Copy link
Contributor

@cjelger cjelger commented Jul 28, 2020

Important: the URL to download the CIF Add-On is set as a CircleCI variable CIF_ADDON_URL. This is obfuscated in the build output so nobody can have access to the add-on.

This has the main drawback that the version of the Add-On is also not visible, so we might forget to update it with each new release of the Add-On. Also because variables are also obfuscated in the CircleCI backend, it might become impossible with time to know which version we currently use.

We could hence maybe rename the variable to CIF_ADDON_URL_2020_7_7_SNAPSHOT so it would be straightforward from the name what version we use. If we release a new version of the Add-On, we would add for example the variable CIF_ADDON_URL_2020_7_28 and update the build.

The drawback here is that we'd have to also update the variable in the it-tests.js file.

Feedback or other ideas is welcome.

@codecov
Copy link

codecov bot commented Jul 28, 2020

Codecov Report

Merging #352 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #352   +/-   ##
=========================================
  Coverage     66.67%   66.67%           
  Complexity      852      852           
=========================================
  Files           192      192           
  Lines          5845     5845           
  Branches        912      912           
=========================================
  Hits           3897     3897           
  Misses         1822     1822           
  Partials        126      126           
Flag Coverage Δ Complexity Δ
#integration 66.86% <ø> (ø) 628.00 <ø> (ø)
#jest 45.70% <ø> (ø) 0.00 <ø> (ø)
#karma 93.93% <ø> (ø) 0.00 <ø> (ø)
#unittests 85.81% <ø> (ø) 821.00 <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 909e24b...661c4b1. Read the comment docs.

Copy link
Contributor

@mhaack mhaack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also remove integration-test-cloudready, AEM Cloud support is only in combination with CIF add-on

@cjelger
Copy link
Contributor Author

cjelger commented Aug 3, 2020

@mhaack I think we could keep integration-test-cloudready so that we test the components on Cloud SDK without the Add-On. In case integration-test-cloudready-with-addon fails, we'd know that the problem comes from the Add-On and not the Cloud SDK. In case both fail, we'd know the problem comes from the Cloud SDK. WDYT?

@mhaack
Copy link
Contributor

mhaack commented Aug 3, 2020

Ok I'm fine with that it might help to find and delimit errors.

@cjelger cjelger added the testing label Aug 3, 2020
@cjelger cjelger merged commit 689539d into master Aug 3, 2020
@cjelger cjelger deleted the CIF-1529 branch August 3, 2020 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants