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

Migrate static checks #8288

Merged
merged 6 commits into from Dec 1, 2023
Merged

Conversation

cmaf
Copy link
Contributor

@cmaf cmaf commented Oct 23, 2023

No description provided.

@cmaf cmaf added the wip Work in Progress (PR incomplete - needs more work or rework) label Oct 23, 2023
@cmaf cmaf requested a review from a team as a code owner October 23, 2023 07:15
@katacontainersbot katacontainersbot added the size/huge Largest and most complex task (probably needs breaking into small pieces) label Oct 23, 2023
@cmaf cmaf changed the title Migrate static checks WIP: Migrate static checks Oct 23, 2023
@cmaf cmaf linked an issue Oct 23, 2023 that may be closed by this pull request
tests/static-checks.sh Outdated Show resolved Hide resolved
@cmaf cmaf added the do-not-merge PR has problems or depends on another label Oct 23, 2023
@cmaf cmaf force-pushed the migrate-static-checks branch 2 times, most recently from d858206 to 3516023 Compare October 24, 2023 07:49

# By default in Golang >= 1.16 GO111MODULE is set to "on",
# some subprojects in this repo may not support "go modules",
# set GO111MODULE to "auto" to enable module-aware mode only when
# a go.mod file is present in the current directory.
export GO111MODULE="auto"
export tests_repo="${tests_repo:-github.com/kata-containers/tests}"
export tests_repo="${tests_repo:-github.com/kata-containers/kata-containers/tests}"
Copy link
Member

Choose a reason for hiding this comment

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

I would drop "repo" from the variable names as this is not technically a repository anymore and use variable names just as test_path and test_dir instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amshinde updated


# Unmerged (U) and Unknown (X) files. These particular filters
# shouldn't be necessary but just in case...
filters+="UX"
Copy link
Member

Choose a reason for hiding this comment

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

man pages indicate that X(Unknown) mostly indicates a bug, I would just drop it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amshinde thanks, I removed this

@cmaf cmaf mentioned this pull request Oct 25, 2023
@cmaf cmaf force-pushed the migrate-static-checks branch 2 times, most recently from f656a35 to 7b09917 Compare November 10, 2023 03:19
@cmaf cmaf force-pushed the migrate-static-checks branch 6 times, most recently from 874b43a to d975ab1 Compare November 16, 2023 21:23
@cmaf cmaf removed do-not-merge PR has problems or depends on another wip Work in Progress (PR incomplete - needs more work or rework) labels Nov 16, 2023
@cmaf cmaf changed the title WIP: Migrate static checks Migrate static checks Nov 16, 2023
@cmaf cmaf added the wip Work in Progress (PR incomplete - needs more work or rework) label Nov 16, 2023
@gkurz
Copy link
Member

gkurz commented Nov 17, 2023

Thanks for this work @cmaf ! FYI I'm looking into eradicating users of clone_tests_repo. I'm interested by the import of install_yq.sh this PR is doing and I'm basing my patches on that. I'll help you on this PR and create one for my work.

@cmaf
Copy link
Contributor Author

cmaf commented Nov 21, 2023

@gkurz thank you! Please link here when you do

@cmaf cmaf removed the wip Work in Progress (PR incomplete - needs more work or rework) label Nov 22, 2023
# Returns the information in format "${filter}\t${file}".
get_pr_changed_file_details()
{
get_pr_changed_file_details_full | grep -v "vendor/"
Copy link
Member

Choose a reason for hiding this comment

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

Improvement suggestion that could go to a followup commit in this PR:

Instead of grepping out the vendor files, what about telling git diff-tree to skip them in the first place. This can be achieved with the addition of a terminal :^*/vendor/* argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gkurz this is a good suggestion but not in scope for this particular PR since it is just concerned with migration

Copy link
Member

Choose a reason for hiding this comment

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

Do you have any plan to reconcile this script with the existing https://github.com/kata-containers/kata-containers/blob/main/ci/install_yq.sh ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gkurz I switched to use that one


[ ! -f "$versions_file" ] && die "cannot find $versions_file"

"${GOPATH}/src/${tests_repo}/install_yq.sh" >&2
Copy link
Member

Choose a reason for hiding this comment

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

${tests_repo} isn't defined and will break. Shouldn't this point to something like "${repo_root_dir}/tests/install_yq.sh" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gkurz this was an oversight on my part, thanks for catching it! Updated with the ci dir one.

Move static checks scripts and dependencies from tests to
kata-containers repo.

Fixes kata-containers#8187

Signed-off-by: Amulyam24 <amulmek1@in.ibm.com>
Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
Signed-off-by: Bin Liu <bin@hyper.sh>
Signed-off-by: Carlos Venegas <jos.c.venegas.munoz@intel.com>
Signed-off-by: Chao Wu <chaowu@linux.alibaba.com>
Signed-off-by: Chelsea Mafrica <chelsea.e.mafrica@intel.com>
Signed-off-by: Dan Middleton <dan.middleton@intel.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Derek Lee <derlee@redhat.com>
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
Signed-off-by: Eric Ernst <eric_ernst@apple.com>
Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
Signed-off-by: Fupan Li <fupan.lfp@antgroup.com>
Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
Signed-off-by: Ganesh Maharaj Mahalingam <ganesh.mahalingam@intel.com>
Signed-off-by: Graham Whaley <graham.whaley@intel.com>
Signed-off-by: Jakob Naucke <jakob.naucke@ibm.com>
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Signed-off-by: Jeremi Piotrowski <jpiotrowski@microsoft.com>
Signed-off-by: Jon Olson <jonolson@google.com>
Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
Signed-off-by: Julio Montes <julio.montes@intel.com>
Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>
Signed-off-by: Manohar Castelino <manohar.r.castelino@intel.com>
Signed-off-by: Marco Vedovati <mvedovati@suse.com>
Signed-off-by: Nitesh Konkar <niteshkonkar@in.ibm.com>
Signed-off-by: Peng Tao <bergwolf@gmail.com>
Signed-off-by: Salvador Fuentes <salvador.fuentes@intel.com>
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
Signed-off-by: Shiming Zhang <wzshiming@foxmail.com>
Signed-off-by: Snir Sheriber <ssheribe@redhat.com>
Signed-off-by: stevenhorsman <steven@uk.ibm.com>
Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Signed-off-by: Xu Wang <xu@hyper.sh>
Signed-off-by: Yang Bo <bo@hyper.sh>
Signed-off-by: Zvonko Kaiser <zkaiser@nvidia.com>
Move the tool as a dependency for static checks migration.

Fixes kata-containers#8187

Signed-off-by: Bin Liu <bin@hyper.sh>
Signed-off-by: Chelsea Mafrica <chelsea.e.mafrica@intel.com>
Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
Signed-off-by: Ganesh Maharaj Mahalingam <ganesh.mahalingam@intel.com>
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Signed-off-by: Julio Montes <julio.montes@intel.com>
Move tool as part of static checks migration.

Fixes kata-containers#8187

Signed-off-by: Bo Chen <chen.bo@intel.com>
Signed-off-by: Carlos Venegas <jos.c.venegas.munoz@intel.com>
Signed-off-by: Chao Wu <chaowu@linux.alibaba.com>
Signed-off-by: Chelsea Mafrica <chelsea.e.mafrica@intel.com>
Signed-off-by: Dan Middleton <dan.middleton@intel.com>
Signed-off-by: Derek Lee <derlee@redhat.com>
Signed-off-by: Eric Ernst <eric.ernst@intel.com>
Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
Signed-off-by: Graham Whaley <graham.whaley@intel.com>
Signed-off-by: Hui Zhu <teawater@antfin.com>
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Signed-off-by: Jimmy Xu <xjmmyshcn@gmail.com>
Signed-off-by: Liu Xiaodong <xiaodong.liu@intel.com>
Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>
Signed-off-by: Shiming Zhang <wzshiming@foxmail.com>
Signed-off-by: Snir Sheriber <ssheribe@redhat.com>
Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Move tool as part of static checks migration.

Fixes kata-containers#8187

Signed-off-by: Chelsea Mafrica <chelsea.e.mafrica@intel.com>
Signed-off-by: Derek Lee <derlee@redhat.com>
Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
Signed-off-by: Graham Whaley <graham.whaley@intel.com>
Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Signed-off-by: Marco Vedovati <mvedovati@suse.com>
Signed-off-by: Peng Tao <bergwolf@hyper.sh>
Signed-off-by: Shiming Zhang <wzshiming@foxmail.com>
Signed-off-by: Snir Sheriber <ssheribe@redhat.com>
Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Updates to scripts for static-checks.sh functionality, including common
functions location, the move of several common functions to the existing
common.bash, adding hadolint and xurls to the versions file, and changes
to static checks for running in the main kata containers repo.

The changes to the vendor check include searching for existing go.mod
files but no other changes to expand the test.

Fixes kata-containers#8187

Signed-off-by: Chelsea Mafrica <chelsea.e.mafrica@intel.com>
@cmaf
Copy link
Contributor Author

cmaf commented Nov 28, 2023

FYI I'm looking into eradicating users of clone_tests_repo. I'm interested by the import of install_yq.sh this PR is doing and I'm basing my patches on that. I'll help you on this PR and create one for my work.

@gkurz are you able to use the one that exists in kata-containers/ci as opposed to the one that would be imported from tests/.ci?

Generate a go.sum file for tests.

Fixes kata-containers#8187

Signed-off-by: Chelsea Mafrica <chelsea.e.mafrica@intel.com>
@gkurz
Copy link
Member

gkurz commented Nov 30, 2023

/test

@cmaf cmaf merged commit 818b8f9 into kata-containers:main Dec 1, 2023
172 of 178 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test size/huge Largest and most complex task (probably needs breaking into small pieces)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move static-checks.sh from tests to kata-containers repo
6 participants