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

Ensure consistency of methods signatures #245

Merged
merged 4 commits into from
Jan 6, 2017
Merged

Conversation

GochoMugo
Copy link
Collaborator

Bug:

The library assumes signatures of methods to be, somewhat:

methodName(requiredParam1, requiredParam2, form = {})

where requiredParam1 (requiredParam2, ..., requiredParamN)
are parameters that MUST be provided, and
form is an optional object allowing supplying any additional,
optional parameters that the Bot API allows.

This allows any new parameters added by Telegram to be
readily-supported by our library.

Notes:

This fix is backwards-compatible; while we should
notify the community of the change in method signatures,
old code using the old method signatures will not
break.

In the future, when we decide to release a major release
(i.e. bump major version), we could remove the code
providing this backwards-compatibility.

@GochoMugo
Copy link
Collaborator Author

I also suggest:

  1. Documenting the old signatures (for completeness)
  2. Deprecation notice to allow users to migrate to new API with time

Should I proceed to include the above additions?

Bug:

  The library assumes signatures of methods to be, somewhat:

    methodName(requiredParam1, requiredParam2, form = {})

  where 'requiredParam1' ('requiredParam2', ..., 'requiredParamN')
  are parameters that MUST be provided, and
  'form' is an optional object allowing supplying any additional,
  optional parameters that the Bot API allows.

  This allows any new parameters added by Telegram to be
  readily-supported by our library.
@GochoMugo GochoMugo force-pushed the fix/methods-consistency branch from b71e553 to e6cfe90 Compare January 6, 2017 16:06
@codecov-io
Copy link

codecov-io commented Jan 6, 2017

Current coverage is 79.28% (diff: 87.50%)

Merging #245 into master will increase coverage by 0.47%

@@             master       #245   diff @@
==========================================
  Files             3          3          
  Lines           538        560    +22   
  Methods          97         97          
  Messages          0          0          
  Branches        100        111    +11   
==========================================
+ Hits            424        444    +20   
- Misses          114        116     +2   
  Partials          0          0          

Powered by Codecov. Last update ce4dff7...4556e2f

@GochoMugo GochoMugo merged commit bc75495 into master Jan 6, 2017
@GochoMugo GochoMugo deleted the fix/methods-consistency branch January 6, 2017 18:18
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.

3 participants