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

USHIFT-642: Static analysis for .go, .sh, and .py #1558

Merged
merged 1 commit into from
Mar 28, 2023

Conversation

pmtk
Copy link
Member

@pmtk pmtk commented Mar 23, 2023

No description provided.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Mar 23, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented Mar 23, 2023

@pmtk: This pull request references USHIFT-642 which is a valid jira issue.

In response to this:

/cc

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 23, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 23, 2023

@pmtk: GitHub didn't allow me to request PR reviews from the following users: pmtk.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 23, 2023
@pmtk pmtk changed the title WIP USHIFT-642: Static analysis for Go and shell scripts USHIFT-642: Static analysis for Go and shell scripts Mar 25, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 25, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented Mar 25, 2023

@pmtk: This pull request references USHIFT-642 which is a valid jira issue.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@pmtk pmtk force-pushed the static-analysis branch 3 times, most recently from 05c7be8 to 7b204ce Compare March 27, 2023 13:53
@pmtk pmtk changed the title USHIFT-642: Static analysis for Go and shell scripts USHIFT-642: Static analysis for .go, .sh, and .py Mar 27, 2023
@pmtk
Copy link
Member Author

pmtk commented Mar 27, 2023

/test e2e-openshift-conformance-reduced-arm

@pmtk
Copy link
Member Author

pmtk commented Mar 27, 2023

/test e2e-router-smoke-test

@dhellmann
Copy link
Contributor

This looks OK. It would be nice if the tools could be run by a developer using a container image, but I suppose that wouldn't work well in CI where the job is already running in a container?

@pmtk
Copy link
Member Author

pmtk commented Mar 28, 2023

This looks OK. It would be nice if the tools could be run by a developer using a container image, but I suppose that wouldn't work well in CI where the job is already running in a container?

Not sure about running container within a job - by glancing https://www.redhat.com/sysadmin/podman-inside-kubernetes it might need some extra privileges that CI probably don't support.
Other way would be to use that container with tools as a base and copy sources into it, e.g.:

    FROM linter-tools
    COPY microshift /tmp/
    WORKDIR /tmp/microshift/

then in a job: cd /tmp/microshift && make lint-xyz - we'd have to find a way to keep this straightforward for dev as well: make lint would start a container with sources mounted (just like rpm-podman).

scripts/fetch_tools.sh Outdated Show resolved Hide resolved
scripts/fetch_tools.sh Outdated Show resolved Hide resolved
@pmtk pmtk force-pushed the static-analysis branch 2 times, most recently from 7faca26 to b959cac Compare March 28, 2023 11:54
@ggiguash
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 28, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 28, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ggiguash, pmtk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@pmtk
Copy link
Member Author

pmtk commented Mar 28, 2023

/test e2e-openshift-conformance-reduced

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 3b55dbd and 2 for PR HEAD 2b06325 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 28, 2023

@pmtk: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit d004dc5 into openshift:main Mar 28, 2023
@pmtk pmtk deleted the static-analysis branch March 29, 2023 13:26
@dhellmann
Copy link
Contributor

dhellmann commented Apr 18, 2023

2b06325 also resolves USHIFT-987

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants