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

Address compiler warning/notice #233

Merged
merged 2 commits into from
Oct 7, 2024
Merged

Conversation

keiravillekode
Copy link
Contributor

No description provided.

@keiravillekode
Copy link
Contributor Author

One exercise has errors with the latest compiler, two have warnings.

We might want to either fix github.com/vlang Test to use the same compiler version as github.com/vlang-test-runner, or update the exercises and test runner for a more recent V compiler version.

Checking allergies exercise...
---- Testing... ----------------------------------------------------------------
temp/run_test.v:197:17: error: sorted_with_compare callback function parameter `left` with type `voidptr` should be `&Allergen`
  195 | }
  196 | 
  197 | fn compare(left voidptr, right voidptr) int {
      |                 ~~~~~~~
  198 |     return int(left) - int(right)
  199 | }
temp/run_test.v:197:32: error: sorted_with_compare callback function parameter `right` with type `voidptr` should be `&Allergen`
  195 | }
  196 | 
  197 | fn compare(left voidptr, right voidptr) int {
      |                                ~~~~~~~
  198 |     return int(left) - int(right)
  199 | }
checker summary: 2 V errors, 0 V warnings, 0 V notices

Checking custom_set exercise...
---- Testing... ----------------------------------------------------------------
OK     302.047 ms /home/runner/work/vlang/vlang/temp/run_test.v
temp/run_test.v:3:1: warning: const () groups will be an error after 2025-01-01 (`v fmt -w source.v` will fix that for you)
--------------------------------------------------------------------------------
    1 | module main
    2 | 
    3 | const (
      | ~~~~~
    4 |     empty         = CustomSet.new([]int{})
    5 |     another_empty = CustomSet.new([]int{})
...
checker summary: 0 V errors, 1 V warnings, 0 V notices

Checking isbn_verifier exercise...
---- Testing... ----------------------------------------------------------------
OK     332.486 ms /home/runner/work/vlang/vlang/temp/run_test.v
temp/isbn_verifier.v:6:1: warning: const () groups will be an error after 2025-01-01 (`v fmt -w source.v` will fix that for you)
    4 | import regex { regex_opt }
    5 | 
    6 | const (
      | ~~~~~
    7 |     isbn_len = 10
    8 |
...
checker summary: 0 V errors, 1 V warnings, 0 V notices

@keiravillekode
Copy link
Contributor Author

We need something like this PR, now that the V test runner has been updated

Without this PR, tests for the Allergies exercise currently fail to compile.
https://exercism.org/tracks/vlang/exercises/allergies/

@kahgoh kahgoh merged commit 757a23f into exercism:main Oct 7, 2024
4 checks passed
@keiravillekode keiravillekode deleted the notice-warning branch October 7, 2024 17:04
@hraftery
Copy link
Contributor

hraftery commented Oct 8, 2024

We might want to either fix github.com/vlang Test to use the same compiler version as github.com/vlang-test-runner

FWIW, this was the motivation behind exercism/vlang-test-runner#10 which was completed (after a brief red herring) here: exercism/vlang-test-runner@31c843f

At the time I wasn't sure how the track worked, so focused on making the test-runner predictable. AFAIK that's working well, and I think you've been able to keep it up to date.

Turning my attention to the track, I see now that it uses a GitHub Action for obtaining v, relying on this: https://github.com/vlang/setup-v

That project seems to have supported a version: key to specify the version of v all along. I wonder if we should include that key here:

uses: vlang/setup-v@f0b3323984699c80284901a9ffd8d3e2997f22b2

Then at least they could be updated in lock-step, and you could test both before committing.

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