Closed mattmccutchen closed 2 months ago
Still on my radar ... although I may not get a chance to look at it until tomorrow or Saturday ;-)
I did need to make one change to this PR to get it to work for me. I suspect it is because I am using an older version (2.7.2) of ruby?
Other than that - good work!
$ git diff
diff --git a/lib/braid/sorbet/fake_runtime.rb b/lib/braid/sorbet/fake_runtime.rb
index 387768c..90452df 100644
--- a/lib/braid/sorbet/fake_runtime.rb
+++ b/lib/braid/sorbet/fake_runtime.rb
@@ -88,7 +88,7 @@ module Braid
define_method(prop_name) {
@attrs[prop_name]
}
- define_method(prop_name.name + '=') { |new_value|
+ define_method(:"#{prop_name}=") { |new_value|
@attrs[prop_name] = new_value
}
end
This gets most files in
lib
totyped: strict
. I also looked briefly at the output of Sorbet's "highlight untyped code" feature and made a few more obvious fixes.Notable large changes:
Replace untyped "options" hashes with keyword arguments (for
Config#initialize
) orT::Struct
subclasses (forMirror::new_from_options
and commands). This required adding barebones support forT::Struct
to the fake Sorbet runtime.Command arguments were passed through
Command::run
as an untyped*args
. We want to have different argument types for each command, but we want to continue to share the wrapper code inCommand::run
across all commands. So have the commands take their arguments ininitialize
instead ofrun
and let the caller callnew
on the desired command class directly instead of havingCommand::run
do it.Notable small changes:
Add
false
default for some command-line options so we can annotate the corresponding variables as non-nilable.Now that we don't want
clear_remote
taking an untyped options hash, it seems clearest to just duplicate the@options.keep
conditional at each caller that has such an option (Add
doesn't, for whatever reason).Remove options from the
Status
command because the CLI never passed them anyway.To resolve a type error that showed up in
Mirror::new_from_options
when we annotated the options, clean upextract_path_from_url
so that it never returns nil. Take advantage ofString#delete_suffix
, which requires Ruby >= 2.5.0, so bump the minimum Ruby version in the gemspec. (I haven't tested that everything else works on Ruby 2.5.0; we currently don't have a good process to ensure the minimum is set correctly.)Fix
Push#config_mode
that was defined in the wrong place, and add a test.The tests pass on my Linux and Windows systems.