Skip to content

Add yard.rbi with root log method#94

Closed
dduugg wants to merge 1 commit intoShopify:mainfrom
dduugg:yard
Closed

Add yard.rbi with root log method#94
dduugg wants to merge 1 commit intoShopify:mainfrom
dduugg:yard

Conversation

@dduugg
Copy link
Copy Markdown
Contributor

@dduugg dduugg commented Aug 8, 2022

Type of Change

  • Add RBI for a new gem
  • Modify RBI for an existing gem
  • Other:

Changes

Adds the log method in an attempt to solve the problem posted here (and pasted below):

I'm attempting to migrate yard-sorbet to use tapioca for RBI generation. WIP PR: dduugg/yard-sorbet#103
I'm following the migration guide: https://github.com/Shopify/tapioca/wiki/Migrating-to-Tapioca
However, typechecking is currently failing:

$ be srb tc                                                                                                                         [15:52:16]
lib/yard-sorbet/sig_to_yard.rb:60: Method log does not exist on T.class_of(YARDSorbet::SigToYARD) https://srb.help/7003
    60 |        log.info("Unsupported sig aref node #{node.source}")
                ^^^

lib/yard-sorbet/sig_to_yard.rb:126: Method log does not exist on T.class_of(YARDSorbet::SigToYARD) https://srb.help/7003
     126 |      log.warn("Unsupported sig #{node.type} node #{node.source}")
                ^^^
Errors: 2
FAIL

log is defined by yard here: https://github.com/lsegal/yard/blob/main/lib/yard/globals.rb#L20-L22
My understanding is that I should be able to resolve this by adding requiring yard/globals in tapioca/require, then invoking bin/tapioca gem yard, which I attempted to do in these commits:
dduugg/yard-sorbet@9e00ecd
dduugg/yard-sorbet@8717134
However, I'm still seeing the same errors when I attempt to invoke srb tc
What am I doing wrong?

@Morriar
Copy link
Copy Markdown
Contributor

Morriar commented Aug 9, 2022

I believe this is going to be fixed by Shopify/tapioca#1102. @dduugg can you confirm? Thanks!

@Morriar Morriar force-pushed the main branch 5 times, most recently from b8b1035 to 5974cd0 Compare August 9, 2022 17:46
@dduugg
Copy link
Copy Markdown
Contributor Author

dduugg commented Aug 9, 2022

@Morriar should be! This benefits slightly from having a type signature, but that's low value. Closing for now, will re-open if the underlying issue isn't fixed in the next Tapioca release.

@dduugg dduugg closed this Aug 9, 2022
@Morriar Morriar added the rbi Change related to RBI annotations label Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rbi Change related to RBI annotations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants