search

grammyjs / grammY

The Telegram Bot Framework.
https://grammy.dev
MIT License
2.17k stars 110 forks source link

refactor: prefer FsFile#readable over std/iterateReader #560

Closed fwqaaq closed 4 months ago

fwqaaq commented 5 months ago
  1. iterateReader was deprecated. See: https://deno.land/std@0.211.0/streams/iterate_reader.ts?s=iterateReader.
  2. Deno.open need to release resources. Deno provided using keywords for this purpose. Like this: https://github.com/denoland/deno/blob/08f46ac4467968f53400c4cf9db95f62b424a161/cli/tsc/dts/lib.deno.ns.d.ts#L1891
  3. importMaps, it would be more convenient, if grammy and grammy_type were to be published JSR. (Like: deno add)
fwqaaq commented 5 months ago

Hey, @roj1512 and @KnorpelSenf, I just tried it out. Using Deno std throudh JSR works perfectly fine, and VSCode Deno plugin also support it.

9AF7D371-08D6-4C98-815A-884C31A3EBE1-82762-0004F95679DE0A5B

KnorpelSenf commented 5 months ago

Hey, @roj1512 and @KnorpelSenf, I just tried it out. Using Deno std throudh JSR works perfectly fine, and VSCode Deno plugin also support it.

That was never the question :)

KnorpelSenf commented 4 months ago

@fwqaaq the tests are failing, something is broken with using. Can you fix this up?

We use other deprecated methods in the tests that cover input files. Do you want to adjust the tests in the same pull request, or should this be done at a later point?

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 85.71429% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 45.76%. Comparing base (4cb8e68) to head (a8beede).

:exclamation: Current head a8beede differs from pull request most recent head 78662fd. Consider uploading reports for the commit 78662fd to get more accurate results

Files Patch % Lines
src/types.deno.ts 85.71% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #560 +/- ## ========================================== - Coverage 46.43% 45.76% -0.67% ========================================== Files 19 19 Lines 6121 5139 -982 Branches 330 331 +1 ========================================== - Hits 2842 2352 -490 + Misses 3276 2784 -492 Partials 3 3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

KnorpelSenf commented 4 months ago

LGTM otherwise, thanks for the patience during these many iterations!

KnorpelSenf commented 4 months ago

@fwqaaq please consider signing your commits in the future. I'll ignore this for now and merge anyway :)

fwqaaq commented 4 months ago

Thanks bro, I remember it.😉

KnorpelSenf commented 4 months ago

@all-contributors add @fwqaaq for idea, code

allcontributors[bot] commented 4 months ago

@KnorpelSenf

I've put up a pull request to add @fwqaaq! :tada: