-
Notifications
You must be signed in to change notification settings - Fork 58
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
Fix "LoadError: cannot load such file" everywhere - Remove dynamic requiring for API methods #323
Comments
rusikf
changed the title
Fix LoadError: cannot load such file everywhere - Remove dynamic requiring for API methods
Fix "LoadError: cannot load such file" everywhere - Remove dynamic requiring for API methods
Apr 25, 2024
I agree. I also want to point out that the meta programming done in this library has multiple issues, including the one mentioned:
module Hubspot
module Discovery
module BaseModuleClient
attr_reader :params
# Defining `initialize` in a module to be included also feels like a
# code smell, at this point why not just use a base class to
# inherit from?
def initialize(params)
@params = params
end
class << self
def included(base)
base.extend(ClassMethods)
end
end
module ClassMethods
def api_modules(*module_names)
module_names.each do |module_name|
# Call code that create methods here
end
end
def api_classes(*class_names)
class_names.each do |module_name|
# Call code that create methods here
end
end
end
end
end
end Then instead of something like: def api_classes
%i[
pipeline_stages
pipeline_stage_audits
pipelines
pipeline_audits
].freeze
end It becomes: api_classes :pipeline_stages,
:pipeline_stage_audits,
:pipelines,
:pipeline_audits I'm sure other solutions also exist. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi Hubspot Team!
On calling most API methods with invalid params We don't have validation because of dynamic requiring files
Idea 💡 - Can we remove the dynamic code required?
The ability to see the raw backtrace/logs/validation errors is much better than LoadError
Note: Everything works if pass valid parameters.
Thanks!
The text was updated successfully, but these errors were encountered: