Skip to content
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

Add HubSpot module alias #140

Merged
merged 1 commit into from
Nov 21, 2018
Merged

Add HubSpot module alias #140

merged 1 commit into from
Nov 21, 2018

Conversation

cbisnett
Copy link
Collaborator

There have been some requests to change the module name to use the stylized HubSpot name which is ultimately a large change for a small stylistic benefit. This change allows developers to use HubSpot or Hubspot without breaking backwards compatibility.

There have been some requests to change the module name to use the
stylized HubSpot name which is ultimately a large change for a small
stylistic benefit. This change allows developers to use HubSpot or
Hubspot without breaking backwards compatibility.
@cbisnett cbisnett mentioned this pull request Nov 21, 2018
@@ -28,3 +28,6 @@ def self.configure(config={})

require 'hubspot/railtie' if defined?(Rails)
end

# Alias the module for those looking to use the stylized name HubSpot
HubSpot = Hubspot
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this 👍 as I would also expect HubSpot instead of Hubspot. Shall we make a ticket to replace Hubspot with HubSpot as part of a v1 release?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually performing the swap touches a bunch of code locations (see #128) for only a minor syntax change. Changing this would also mean that anyone who has been using this gem in their own projects would have to change every instance of Hubspot to HubSpot. This change means that either will work and I don't think it's too ambiguous about what module you're trying to reference. There is precedence for supporting these types of things, for example RestClient allows you to import it using require "restclient", require "rest-client", or require "rest_client" (https://github.com/rest-client/rest-client/tree/master/lib)

Copy link
Contributor

@SViccari SViccari Nov 21, 2018

Choose a reason for hiding this comment

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

Neat, I didn't know that about rest-client. I should clarify that I'm on board with making this change now so users can choose between HubSpot and Hubspot.

I would also be on board with making the larger change of swapping out Hubspot to HubSpot, even though the change would touch a large number of files. To help users continue to upgrade without having to replaces their existing code to use the new HubSpot namespace, we could replace this assignment with the inverse: Hubspot = HubSpot.

I don't feel strongly about it :) I Just figured I'd bring it up as an option (and future endeavor after we tackle the more pressing work) to help promote consistency and eventually remove the need for this "alias".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good. I don't feel strongly about it either way but I figure a change that requires less work from downstream users is probably preferable.

@cbisnett cbisnett merged commit ad9da55 into master Nov 21, 2018
@cbisnett cbisnett deleted the cdb_hubspot_alias branch November 21, 2018 17:58
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.

2 participants