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

Integrate codespell on toolbox code #1149

Merged
merged 1 commit into from
Nov 19, 2022

Conversation

nievesmontero
Copy link
Collaborator

No description provided.

@softwarefactory-project-zuul
Copy link

Build succeeded.

✔️ unit-test SUCCESS in 7m 36s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 7m 36s
✔️ system-test-fedora-rawhide SUCCESS in 33m 26s
✔️ system-test-fedora-36 SUCCESS in 10m 30s
✔️ system-test-fedora-35 SUCCESS in 10m 29s

Copy link
Member

@debarshiray debarshiray left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, @nievesmontero

src/cmd/completion.go Outdated Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
meson.build Show resolved Hide resolved
@softwarefactory-project-zuul
Copy link

Build succeeded.

✔️ unit-test SUCCESS in 7m 42s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 7m 41s
✔️ system-test-fedora-rawhide SUCCESS in 27m 37s
✔️ system-test-fedora-36 SUCCESS in 10m 01s
✔️ system-test-fedora-35 SUCCESS in 11m 00s

debarshiray pushed a commit to debarshiray/toolbox that referenced this pull request Nov 17, 2022
Copy link
Member

@debarshiray debarshiray left a comment

Choose a reason for hiding this comment

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

I split the commit that fixes the existing spelling mistakes into #1166 and merged it.

@softwarefactory-project-zuul
Copy link

Build succeeded.

✔️ unit-test SUCCESS in 8m 58s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 8m 46s
✔️ system-test-fedora-rawhide SUCCESS in 26m 10s
✔️ system-test-fedora-36 SUCCESS in 11m 27s
✔️ system-test-fedora-35 SUCCESS in 13m 06s

Copy link
Member

@debarshiray debarshiray left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! Could you please remove this patch from this pull request? Like I said before, it has already been merged via #1166

@debarshiray
Copy link
Member

Could you please remove this patch from this pull request? Like I
said before, it has already been merged via #1166

Try git rebase -i main while sitting inside your branch and remove the line describing this patch.

@debarshiray
Copy link
Member

recheck

Copy link
Member

@debarshiray debarshiray left a comment

Choose a reason for hiding this comment

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

Nitpick: you can drop this line from your commit message:

Codespell has been added to the meson.build file and has been tested.

... because that's obvious from looking at the commit.

meson.build Outdated Show resolved Hide resolved
@softwarefactory-project-zuul
Copy link

Build succeeded.

✔️ unit-test SUCCESS in 8m 39s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 8m 40s
✔️ system-test-fedora-rawhide SUCCESS in 31m 34s
✔️ system-test-fedora-36 SUCCESS in 11m 32s
✔️ system-test-fedora-35 SUCCESS in 12m 22s

@softwarefactory-project-zuul
Copy link

Build failed.

unit-test RETRY_LIMIT in 8m 16s
unit-test-migration-path-for-coreos-toolbox RETRY_LIMIT in 8m 32s
system-test-fedora-rawhide RETRY_LIMIT in 31m 16s
system-test-fedora-36 RETRY_LIMIT in 8m 12s
system-test-fedora-35 RETRY_LIMIT in 8m 12s

@debarshiray
Copy link
Member

debarshiray commented Nov 18, 2022

The tests are failing with:

meson.build:62: WARNING: Project targeting '>= 0.58.0' but tried
    to use feature deprecated since '0.56.0': meson.source_root. use
    meson.project_source_root() or meson.global_source_root() instead.

meson.build:62:2: ERROR: Fatal warnings enabled, aborting

Copy link
Member

@debarshiray debarshiray left a comment

Choose a reason for hiding this comment

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

Thanks for the updates, @nievesmontero !

meson.build Outdated
@@ -57,6 +58,10 @@ if shellcheck.found()
test('shellcheck toolbox (deprecated)', shellcheck, args: [toolbox_sh])
endif

if codespell.found()
test('codespell toolbox', codespell, args: ['--skip', meson.project_build_root(), meson.source_root()])
Copy link
Member

@debarshiray debarshiray Nov 18, 2022

Choose a reason for hiding this comment

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

It should be meson.project_source_root(), not meson.source_root(). See the test failures.

@softwarefactory-project-zuul
Copy link

Build failed.

unit-test FAILURE in 8m 28s
unit-test-migration-path-for-coreos-toolbox FAILURE in 8m 41s
✔️ system-test-fedora-rawhide SUCCESS in 36m 33s
✔️ system-test-fedora-36 SUCCESS in 11m 22s
✔️ system-test-fedora-35 SUCCESS in 12m 07s

@debarshiray
Copy link
Member

Thanks for the updates, @nievesmontero !

Build failed.

x unit-test FAILURE in 8m 28s x unit-test-migration-path-for-coreos-toolbox FAILURE in 8m 41s

Now that the codespell test is actually getting run, it's failing:

TASK [Test]
ci-node-36 | ninja: Entering directory `/home/zuul-worker/src/github.com/containers/toolbox/builddir'
ci-node-36 | ninja: no work to do.
ci-node-36 | 1/6 shellcheck src/go-build-wrapper OK              0.04s
ci-node-36 | 2/6 go fmt                          OK              0.05s
ci-node-36 | 3/6 shellcheck profile.d/toolbox.sh OK              0.06s
ci-node-36 | 4/6 codespell toolbox               FAIL            0.33s   exit status 65
ci-node-36 | >>> MALLOC_PERTURB_=94 /usr/bin/codespell --skip /home/zuul-worker/src/github.com/containers/toolbox/builddir /home/zuul-worker/src/github.com/containers/toolbox
ci-node-36 |
ci-node-36 | 5/6 shellcheck toolbox (deprecated) OK              1.05s
ci-node-36 | 6/6 toolbox go unit tests           OK              2.73s
ci-node-36 |
ci-node-36 |
ci-node-36 | Ok:                 5
ci-node-36 | Expected Fail:      0
ci-node-36 | Fail:               1
ci-node-36 | Unexpected Pass:    0
ci-node-36 | Skipped:            0
ci-node-36 | Timeout:            0
ci-node-36 |
ci-node-36 | Full log written to /home/zuul-worker/src/github.com/containers/toolbox/builddir/meson-logs/testlog.txt
ci-node-36 | ERROR
ci-node-36 | {
ci-node-36 |   "delta": "0:00:03.005543",
ci-node-36 |   "end": "2022-11-18 16:41:18.320047",
ci-node-36 |   "msg": "non-zero return code",
ci-node-36 |   "rc": 1,
ci-node-36 |   "start": "2022-11-18 16:41:15.314504"
ci-node-36 | }

@softwarefactory-project-zuul
Copy link

Build failed.

unit-test FAILURE in 8m 37s
unit-test-migration-path-for-coreos-toolbox FAILURE in 8m 38s
✔️ system-test-fedora-rawhide SUCCESS in 37m 35s
✔️ system-test-fedora-36 SUCCESS in 14m 52s
✔️ system-test-fedora-35 SUCCESS in 16m 17s

@debarshiray
Copy link
Member

debarshiray commented Nov 19, 2022

Now that the codespell test is actually getting run, it's failing:

It's because of that "complet" prefix, and a spelling mistake in the bats-support Git submodule in test/system/libs/bats-support.

I do wish codespell offered inline directives to ignore such one-off cases.

@softwarefactory-project-zuul
Copy link

Build succeeded.

✔️ unit-test SUCCESS in 8m 22s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 8m 23s
✔️ system-test-fedora-rawhide SUCCESS in 33m 44s
✔️ system-test-fedora-36 SUCCESS in 11m 13s
✔️ system-test-fedora-35 SUCCESS in 12m 03s

@debarshiray debarshiray merged commit 9438db2 into containers:main Nov 19, 2022
@debarshiray
Copy link
Member

Now that the codespell test is actually getting run, it's failing:

It's because of that "complet" prefix, and a spelling mistake in the bats-support Git submodule in test/system/libs/bats-support.

I do wish codespell offered inline directives to ignore such one-off cases.

I took the liberty to fix this up, and extended the test to cover filenames and hidden files too.

@debarshiray
Copy link
Member

Thanks for working on this, @nievesmontero !

@nievesmontero nievesmontero deleted the codespell branch October 3, 2023 15:54
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