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

🌱 cron: make CSV header optional (3/n) #2261

Merged
merged 3 commits into from
Sep 14, 2022

Conversation

spencerschrock
Copy link
Contributor

Signed-off-by: Spencer Schrock sschrock@google.com

What kind of change does this PR introduce?

Part of a series of refactors to generalize the cron infrastructure to enable re-use by the ossf criticality score project

What is the current behavior?

CSV files read by the controller needed to have a header line:

repo,metadata

What is the new behavior (if this is a feature change)?**

The header line is now optional, and is skipped by the data.csvIterator.

  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

Follow up PR for #2235

Special notes for your reviewer

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

NONE

Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: Spencer Schrock <sschrock@google.com>
@codecov
Copy link

codecov bot commented Sep 13, 2022

Codecov Report

Merging #2261 (6dc366b) into main (bde0ae1) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2261   +/-   ##
=======================================
  Coverage   44.45%   44.45%           
=======================================
  Files          95       95           
  Lines        7987     7987           
=======================================
  Hits         3551     3551           
  Misses       4173     4173           
  Partials      263      263           

@spencerschrock
Copy link
Contributor Author

@oliverchang @calebbrown fyi

Copy link
Contributor

@azeemshaikh38 azeemshaikh38 left a comment

Choose a reason for hiding this comment

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

LGTM with comments.

cron/internal/data/iterator.go Outdated Show resolved Hide resolved
cron/internal/data/iterator.go Outdated Show resolved Hide resolved
Signed-off-by: Spencer Schrock <sschrock@google.com>
@azeemshaikh38 azeemshaikh38 merged commit 2231d1f into ossf:main Sep 14, 2022
@spencerschrock spencerschrock deleted the csv-header branch September 14, 2022 16:39
N8BWert pushed a commit to N8BWert/scorecard that referenced this pull request Sep 23, 2022
* Make CSV header optional.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Appease linter.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Address PR feedback.

Signed-off-by: Spencer Schrock <sschrock@google.com>

Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: N8BWert <n8.wert.b@gmail.com>
N8BWert pushed a commit to N8BWert/scorecard that referenced this pull request Nov 28, 2022
* Make CSV header optional.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Appease linter.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Address PR feedback.

Signed-off-by: Spencer Schrock <sschrock@google.com>

Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: N8BWert <n8.wert.b@gmail.com>
Signed-off-by: nathaniel.wert <nathaniel.wert@kudelskisecurity.com>
N8BWert pushed a commit to N8BWert/scorecard that referenced this pull request Nov 28, 2022
* Make CSV header optional.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Appease linter.

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Address PR feedback.

Signed-off-by: Spencer Schrock <sschrock@google.com>

Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: N8BWert <n8.wert.b@gmail.com>
Signed-off-by: nathaniel.wert <nathaniel.wert@kudelskisecurity.com>
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.

2 participants