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

Remove http method from Base module. #5

Closed
1 of 2 tasks
pkrawat1 opened this issue Dec 19, 2017 · 1 comment
Closed
1 of 2 tasks

Remove http method from Base module. #5

pkrawat1 opened this issue Dec 19, 2017 · 1 comment

Comments

@pkrawat1
Copy link
Member

pkrawat1 commented Dec 19, 2017

Refer stripe.ex

{api_key, ""} passing this parameter as empty string does not make sense here.

defp commit(method, path, params, opts) do
    config = Keyword.fetch!(opts, :config)
    # TODO: credentials should be investigated why it is {api_key, ""}
    # Did to mimic the earlier behaviour.
    method
      |> http("#{@base_url}/#{path}", params, credentials: {config[:api_key], ""})
      |> respond
  end

Please do necessary investigation for this one and fix it.

Edit:

  • Remove http and money utils from Base
  • Remove random* methods from Bogus
@pkrawat1 pkrawat1 self-assigned this Dec 21, 2017
@pkrawat1 pkrawat1 added this to the Release 1.5.0 milestone Dec 21, 2017
@oyeb
Copy link
Contributor

oyeb commented Dec 21, 2017

There is no usage of the http method defined in base.ex.
Hence, I'm changing the purpose of this issue.

@oyeb oyeb changed the title Passing params to http post method we are passing 2 params; the use of second empty string is unknown. Remove http method from Base module. Dec 21, 2017
@pkrawat1 pkrawat1 removed this from the Release 1.5.0 milestone Jan 25, 2018
@oyeb oyeb assigned oyeb and unassigned pkrawat1 Jan 25, 2018
@oyeb oyeb added this to the 1.1.0-beta milestone Jan 25, 2018
oyeb added a commit that referenced this issue Jan 30, 2018
* Fixes #5
* Moved the stripe_test to integration.
* Fixed credo issue in money integration test
oyeb added a commit that referenced this issue Mar 20, 2018
* Fixes #5
* Moved the stripe_test to integration.
* Fixed credo issue in money integration test

Changes to MONEI
* Removed unnecessary clauses from MONEI
* Re-formatted source.
oyeb added a commit that referenced this issue Mar 20, 2018
* Fixes #5 and #109
* Moved the stripe_test to integration.
* Fixed credo issue in money integration test

Changes to MONEI
* Removed unnecessary clauses from MONEI
* Re-formatted source.
ashish173 pushed a commit that referenced this issue Mar 21, 2018
* Fixes #5 and #109
* Moved the stripe_test to integration.
* Fixed credo issue in money integration test

Changes to MONEI
* Removed unnecessary clauses from MONEI
* Re-formatted source.
ashish173 pushed a commit that referenced this issue Apr 22, 2018
* Fixes #5 and #109
* Moved the stripe_test to integration.
* Fixed credo issue in money integration test

Changes to MONEI
* Removed unnecessary clauses from MONEI
* Re-formatted source.
ashish173 pushed a commit that referenced this issue Apr 22, 2018
* Fixes #5 and #109
* Moved the stripe_test to integration.
* Fixed credo issue in money integration test

Changes to MONEI
* Removed unnecessary clauses from MONEI
* Re-formatted source.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants