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

Harmonize check and config options #10488

Merged
merged 2 commits into from
Oct 26, 2021
Merged

Harmonize check and config options #10488

merged 2 commits into from
Oct 26, 2021

Conversation

fanny-jiang
Copy link
Contributor

What does this PR do?

  • Removes extraneous report_timing from the check. It wasn't listed in the config and was never used.
  • Adds appcenter config option to the spec.yaml. It was in the check, but the option was never listed in the conf.yaml.example

appcenter refers to whether or not the certs being used are appcenter user certs: datacenter/pyaci@8c5b61f

Motivation

Bug fix

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@codecov
Copy link

codecov bot commented Oct 25, 2021

Codecov Report

Merging #10488 (4f2053c) into master (eca1321) will increase coverage by 0.00%.
The diff coverage is n/a.

Flag Coverage Δ
cisco_aci 95.88% <ø> (+0.03%) ⬆️

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

kayayarai
kayayarai previously approved these changes Oct 25, 2021
Copy link
Contributor

@kayayarai kayayarai left a comment

Choose a reason for hiding this comment

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

Edit suggestions but otherwise 👍

@@ -86,6 +86,12 @@ files:
If you want to keep the cert in a separate file, enter the file path here
value:
type: string
- name: appcenter
description: |
Whether or not appcenter user certificates are being used.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Whether or not appcenter user certificates are being used.
Whether App Center user certificates are being used.

@@ -119,6 +119,11 @@ instances:
#
# cert_key_path: <CERT_KEY_PATH>

## @param appcenter - boolean - optional - default: false
## Whether or not appcenter user certificates are being used.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Whether or not appcenter user certificates are being used.
## Whether App Center user certificates are being used.

@fanny-jiang fanny-jiang merged commit f5c3c0e into master Oct 26, 2021
@fanny-jiang fanny-jiang deleted the fanny/cisco_aci branch October 26, 2021 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants