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

New Swimm Units #850

Merged
merged 1 commit into from
Nov 11, 2020
Merged

Conversation

shreyamalviya
Copy link
Contributor

Adds 3 new Swimm units for a new playlist (Adding a PBA in 5 Simple Steps, for Open Sauced):

  • Define what your new PBA does
  • Implement a new PBA — ScheduleJobs
  • Add details about your new PBA

@codecov
Copy link

codecov bot commented Oct 6, 2020

Codecov Report

Merging #850 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #850   +/-   ##
========================================
  Coverage    60.52%   60.52%           
========================================
  Files          165      165           
  Lines         4948     4948           
========================================
  Hits          2995     2995           
  Misses        1953     1953           

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 53f3625...e3b3ad8. Read the comment docs.

},
"app_version": "0.1.90",
"file_version": "1.0.2"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Newline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, Swimm committed these files. I reckon we should let these be just for consistency? Otherwise we'll have to edit the Swimm commit and add a newline every time.

@@ -0,0 +1,31 @@
{
"id": "VW4rf3AxRslfT7lwaug7",
"name": "Implement a new PBA — `ScheduleJobs`",
Copy link
Contributor

Choose a reason for hiding this comment

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

don't we already have this PBA? Maybe we should instead start with a more trivial task, like a PBA that adds a text tile to temp dir?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we can add more playlists/units for trivial tasks; this one was used in the OpenSauced presentation so we should probably keep this separate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this doesn't require the swimmer to add the actual job scheduling commands. They're only required to fetch the commands from an already existing function, get_commands_to_schedule_jobs, which is mentioned in the unit's description. So it's not really hard.

Copy link
Contributor

@VakarisZ VakarisZ left a comment

Choose a reason for hiding this comment

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

Can't approve until questions are answered

Copy link
Contributor

@VakarisZ VakarisZ left a comment

Choose a reason for hiding this comment

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

Approved, you can merge if that's all you wanted to add in this PR

@shreyamalviya shreyamalviya merged commit 4b1b6f8 into guardicore:develop Nov 11, 2020
@shreyamalviya shreyamalviya deleted the swimm-opensauced branch November 11, 2020 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants