search

gchq / gaffer-experimental

Apache License 2.0
6 stars 7 forks source link

GH-288 Unified Helm Chart + Move UI to Gaffer-as-a-Service #304

Closed rocky341 closed 2 years ago

rocky341 commented 2 years ago

Hi @t92549 @GCHQDeveloper314

This PR is being raised to replace #295

Tickets: #288

codecov-commenter commented 2 years ago

Codecov Report

Merging #304 (834a92e) into master (f4e900f) will decrease coverage by 0.03%. The diff coverage is 98.13%.

@@             Coverage Diff              @@
##             master     #304      +/-   ##
============================================
- Coverage     83.97%   83.94%   -0.04%     
  Complexity      246      246              
============================================
  Files           119      119              
  Lines          2764     2765       +1     
  Branches        371      371              
============================================
  Hits           2321     2321              
- Misses          402      403       +1     
  Partials         41       41
Flag Coverage Δ
unittests 68.33% <50.00%> (-0.08%) :arrow_down:

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

Impacted Files Coverage Δ
...ov/gchq/gaffer/gaas/sidecar/CorsConfiguration.java 0.00% <ø> (ø)
...v/gchq/gaffer/gaas/sidecar/SidecarApplication.java 25.00% <0.00%> (-8.34%) :arrow_down:
...uk/gov/gchq/gaffer/gaas/sidecar/SpringContext.java 0.00% <ø> (ø)
...q/gaffer/gaas/sidecar/controller/CustomFilter.java 73.52% <ø> (ø)
...fer/gaas/sidecar/controller/SidecarController.java 100.00% <ø> (ø)
...ava/uk/gov/gchq/gaffer/gaas/sidecar/AppConfig.java 0.00% <ø> (ø)
...ov/gchq/gaffer/gaas/sidecar/CorsConfiguration.java 0.00% <ø> (ø)
...v/gchq/gaffer/gaas/sidecar/SidecarApplication.java 33.33% <ø> (ø)
...uk/gov/gchq/gaffer/gaas/sidecar/SpringContext.java 0.00% <ø> (ø)
...q/gaffer/gaas/sidecar/controller/CustomFilter.java 100.00% <ø> (ø)
... and 102 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 f4e900f...834a92e. Read the comment docs.

GCHQDeveloper314 commented 2 years ago

Could you explain how this PR differs from #295? Is it the same but with merge conflicts removed? That's the implication from the comment closing #295. Ideally the previous PR should have been corrected to fix any conflicts, rather than a new one being opened. This PR is not ready for review as there's a failing check.

rocky341 commented 2 years ago

Could you explain how this PR differs from #295? Is it the same but with merge conflicts removed? That's the implication from the comment closing #295. Ideally the previous PR should have been corrected to fix any conflicts, rather than a new one being opened. This PR is not ready for review as there's a failing check.

The #295 merge conflicts are to do with the moving of GaaS-UI into the Gaffer-as-a-Service directory. That move is also done in this PR. The main difference is that the implementation of the feature this PR intends to introduce - #288 - is different from the approach in #295. This solution uses helm subcharts whereas #295 essentially created one massive helm chart.

t92549 commented 2 years ago

Overall, a lot of files moved so it is hard to review in depth. The only thing to ask is, is the top level gaas README still relevant/correct? Also perhaps the top level README could also do with a change? Also could tools be moved into the same directory? Just put it all under one roof?

Never mind, just seen https://github.com/gchq/gaffer-experimental/pull/312