Skip to content

Conversation

@HenrikBengtsson
Copy link
Contributor

No description provided.

@dirkschumacher
Copy link

dirkschumacher commented Dec 16, 2025

LGTM, but I am not a maintainer 😅. There are some tests for webr-workers, so maybe there is a way to test this feature.

Error: 595:72 error Type number trivially inferred from a number literal, remove type annotation @typescript-eslint/no-inferrable-types
@HenrikBengtsson
Copy link
Contributor Author

HenrikBengtsson commented Dec 17, 2025

The GA build-webr checks produced:

/__w/webr/webr/src/webR/webr-worker.ts
Error:   595:72  error  Type number trivially inferred from a number literal, remove type annotation  @typescript-eslint/no-inferrable-types

I address that in commit 1f2011e. @georgestagg , could please trigger a rerun?

@georgestagg
Copy link
Member

Yeah, the linter is configured to be quite picky…

You may be able to check it locally quicker by cd-ing into src and running make lint

@HenrikBengtsson
Copy link
Contributor Author

You may be able to check it locally quicker by cd-ing into src and running make lint

Thx - worked like a charm. Confirming I could reproduce the lint error, which is gone in the most recent commit.

Copy link
Member

@georgestagg georgestagg left a comment

Choose a reason for hiding this comment

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

LGTM!

If we wanted, we could add a test for download.file() in the same place as we already have a couple of tests that depend on networking here.

Happy to merge without, but it's an option if you'd like to add it. We'd need to test downloading a file from with a reasonable URL that will always be consistent and available.

@HenrikBengtsson
Copy link
Contributor Author

I've added direct tests of utils::download.file() with and without URL redirects.

I also added a test of <shim>::install.packages() from R-universe, which (currently) involves a URL redirect.

@HenrikBengtsson
Copy link
Contributor Author

Forgot to say, I finally got the build environment up so I could run:

./configure && make 
make check

which passes with all OK.

@georgestagg georgestagg merged commit 53f7d6a into r-wasm:main Dec 19, 2025
4 checks passed
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