Codecov Report

Merging #166 into master will decrease coverage by 4.65%.
The diff coverage is 53.7%.

@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
- Coverage   66.98%   62.33%   -4.66%     
==========================================
  Files          36       47      +11     
  Lines        2732     3828    +1096     
==========================================
+ Hits         1830     2386     +556     
- Misses        665     1052     +387     
- Partials      237      390     +153

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 c81a498...f51fc3c. Read the comment docs.

Duuddee! Nice, I'm going to have to review this at one point. :D

BTW, this should have been a bit separated into multiple PRs if possible. :D Like, the worker view, then the pipeline stuff and so on and so forth. :)

@Skarlso I thought about splitting this PR up in multiple PRs beforehand but this was almost impossible. The way this has been implemented now is a bit different from what we agreed on in the RFC since some things didn't work well or included some traps we didn't think of. 😭

Let me check if I can split this up when it's finished. 😄

@michelvocks It's fine, it will just be harder to review. :D

Ah, would you mind updating the RFC in that case? :P

@Skarlso Tests are looking fine now. The test coverage will be decreased a bit but mainly because codecov sucks 😞 I touched a few files where we don't have any tests and codecov simply counts them now as new code 🙄

The plan is to finish the documentation part within the next week. Would be great if you could have another look until then. 😄

@Skarlso sure thing dude. :) It's kind of hard to review 7700 lines of code change though... :D

Noice!

@michelvocks Is there a was to delete the default tags? I mean what if my worker isn't supporting the Python environment? :)

@michelvocks Also, the pipelines need a way to display their affiliated tags. I can't view what environment my pipeline needs once I set a tag. I thought that maybe we'd need the ability to edit tags, but I reconsidered. In case an environment need changes for a pipeline we'd just delete it and re-create it. But we still need an ability to see them tags. :)

@Skarlso sure thing dude. :) It's kind of hard to review 7700 lines of code change though... :D

I don't expect you to review all the lines of code. 😄
It already helps a lot if you just have a general look and may provide feedback like above. Super helpful 🤗

@michelvocks Is there a was to delete the default tags? I mean what if my worker isn't supporting the Python environment? :)

I've added the possibility to provide "negative tags". So if you provide a tag like "-golang" it will remove automatically the "golang" tag (and the "-golang" tag). Not the best solution in terms of user experience but should work for this use-case. 😄

@michelvocks Also, the pipelines need a way to display their affiliated tags. I can't view what environment my pipeline needs once I set a tag. I thought that maybe we'd need the ability to edit tags, but I reconsidered. In case an environment need changes for a pipeline we'd just delete it and re-create it. But we still need an ability to see them tags. :)

Whops, yeah. I added the tags to the overview page.

@michelvocks awesome, I'm okay with the negative tags. :D so do you tag a pipeline automatically when it's created with the appropriate environment tag? Like a Go pipeline gets go a Python pipeline gets Python etc?

@michelvocks awesome, I'm okay with the negative tags. :D so do you tag a pipeline automatically when it's created with the appropriate environment tag? Like a Go pipeline gets go a Python pipeline gets Python etc?

Yep: https://github.com/gaia-pipeline/gaia/pull/166/files#diff-f88a32f252fb4994c4b7249afcc79b6cR57

@Skarlso Docs PR: gaia-pipeline/docs#11

Still WIP. Need to update the RFC.

@Skarlso Docs PR should be fine now. In my opinion, this is now ready for merge.

Let me know what you think. 🤗

@michelvocks Well. Functionality wise this looks okay. I tested it and it worked. :D But man, the code needs refactoring. :DDD Create a bunch of follow up tickets so we can track the most common ones? Like the functions are too long and such? Otherwise, go ahead. :)

@michelvocks Well. Functionality wise this looks okay. I tested it and it worked. :D But man, the code needs refactoring. :DDD Create a bunch of follow up tickets so we can track the most common ones? Like the functions are too long and such? Otherwise, go ahead. :)

Is it only the length of some functions? In my opinion, it is quite common and normal to have a bit longer functions. It makes it often more readable because you can simply follow one block of code instead of switching between several functions. I also know other big projects (Kubernetes, Vault, Terraform etc.) which have bigger functions and they also discussed this and decided it can be beneficial.

ping @Skarlso

I disagree with that. That is why cyclomatic complexity is a thing. And that is why lingers are saying that your functions is too long. It's harder to track down a bug in a big ass function than in a small function where it's basically clear at one quick look what it does. If I have scroll down several pages and still be in the same function I'll loose track of what the beginning of the function was even trying to do in the first place. 😂 Also if a function is huge it's more likely that it does not adhere to SRP. The single responsibility principle. It is likely however that its trying to do everything / too much of its own.

I approve but we have to refractor these into easily tracakble understandable fragments I think.
😊

Also, sorry for the late response, I'm on vacation in Austria. 😁

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 53.70066% with 563 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@c81a498). Learn more about missing BASE report.
Report is 68 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff            @@
##             master     #166   +/-   ##
=========================================
  Coverage          ?   62.33%           
=========================================
  Files             ?       47           
  Lines             ?     3828           
  Branches          ?        0           
=========================================
  Hits              ?     2386           
  Misses            ?     1052           
  Partials          ?      390           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.