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

Support 'password' in config, interchangeable with access_token #145

Conversation

chrisbutcher
Copy link
Contributor

Part of #104

  • Adds password to Configure command workflow, which holds the same value sent to Shopify under the header X-Shopify-Access-Token.
    • The logic which allows either to be used interchangeably is here.
    • The naming of this header is a source of confusion. In the previous themes gem, and in previous incarnations of config.yml, the term password is used. It's also used within the Shopify Private Apps UI, as seen in the related issue above.
  • This PR adds support for password as the preferred flag name, and config.yml key. Eventually we should deprecate the term access_token themekit (a warning is added when calling theme configure with -access_token=foo).

Following my configuration refactor PR shipping, a lot of the duplication still visible in parts of this PR will be cleaned up. #135

Please review, @ilikeorangutans cc: @stevebosworth @cshold

@chrisbutcher chrisbutcher force-pushed the support_password_config_var_interchangeable_with_access_token branch from d401c17 to 63848db Compare February 24, 2016 22:42
@chrisbutcher chrisbutcher force-pushed the support_password_config_var_interchangeable_with_access_token branch from 63848db to d60ee56 Compare February 24, 2016 22:45
@@ -54,9 +57,12 @@ func (conf Configuration) Initialize() (Configuration, error) {

if len(conf.Domain) == 0 {
return conf, fmt.Errorf("missing domain")
} else if !strings.HasSuffix(conf.Domain, "myshopify.com") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we allow .com and .io?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea!

@ilikeorangutans
Copy link
Contributor

One minor comment, otherwise 👍

@chrisbutcher
Copy link
Contributor Author

Will 🚢 this this week, and create a new release at that time

chrisbutcher added a commit that referenced this pull request Mar 9, 2016
…rchangeable_with_access_token

Support 'password' in config, interchangeable with access_token
@chrisbutcher chrisbutcher merged commit e5b4b2a into master Mar 9, 2016
@chrisbutcher chrisbutcher deleted the support_password_config_var_interchangeable_with_access_token branch March 9, 2016 15:57
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