Skip to content

Conversation

@kitebuggy
Copy link

Fixed some typos and possible usability errors.
(Working on extending functionality in the next pull request)

Fixed some typos and possible usability errors.
@edtjones
Copy link

edtjones commented Dec 6, 2015

Amazing - thanks for this. Will review and merge ASAP.

Ed

On 6 Dec 2015, at 14:34, Jason notifications@github.com wrote:

Fixed some typos and possible usability errors.
(Working on extending functionality in the next pull request)

You can view, comment on, or merge this pull request online at:

#1

Commit Summary

Tidied up some minor errors
File Changes

M .gitignore (6)
D .ruby-gemset (1)
D .ruby-version (1)
M README.md (14)
M lib/voipfone_client/registered_mobile.rb (6)
M lib/voipfone_client/sms.rb (2)
Patch Links:

https://github.com/errorstudio/voipfone_client/pull/1.patch
https://github.com/errorstudio/voipfone_client/pull/1.diff

Reply to this email directly or view it on GitHub.

@edtjones
Copy link

edtjones commented Dec 6, 2015

Hi @kitebuggy just a quick question: why remove the .ruby-version and .ruby-gemset files? I find it easier to develop gems with a local gemset and obviously it doesn't affect the gem itself.

Thoughts?

@kitebuggy
Copy link
Author

Hi Ed,

Wow, that was quick!  Thanks for taking the time to respond.

I'm adding support for Extensions (more or less done for my needs) and Reports (in progress), to allow me to automate the collection of our call records, which is what prompted my work on this.

I'm new to gem forking/pulling, so please don't hesitate to let me know if I'm doing anything wrong, or can improve in how I deliver the changes.

As for your comment below, I imagine that you're also an RVM user, hence the use of the .ruby-* files.  I love this functionality, but when working on the gem, I felt these made unrealistic expectations upon my working environment (e.g. I'm working in Ruby 2.1.2 and my version of 2.0.0 is at a different patch level).  By removing these two files from the repo and adding them to the .gitignore file, you and I (and anyone else for that matter) can their own preference files without interfering with each other.  You can copy yours back into place and they should be ignored by git during future updates.

Does this make sense?  I've done something similar with database.yml in RoR deployments, to prevent my local passwords and settings leaking out to the external webserver, for example.

I'd welcome your thoughts.

Kind regards,

Jason 
-- 
Jason Holloway
M. 07887 943 600

On 6 December 2015 at 16:50:02, Ed Jones (notifications@github.com) wrote:

Hi @kitebuggy just a quick question: why remove the .ruby-version and .ruby-gemset files? I find it easier to develop gems with a local gemset and obviously it doesn't affect the gem itself.

Thoughts?


Reply to this email directly or view it on GitHub.

@kitebuggy
Copy link
Author

Hi Ed,

I've added support for extensions and reporting. I've also moved to symbols for the JSON parsing to make it easier to extend and keep in a Ruby-esque tone.

I'm a tad new to this pull malarkey, so I hope I've done this right. Would you confirm, please?

Thanks,

Jason

@kitebuggy kitebuggy closed this Dec 13, 2015
@kitebuggy kitebuggy reopened this Dec 13, 2015
@edtjones
Copy link

Ace! Will look ASAP :-)

On 13 Dec 2015, at 21:30, Jason notifications@github.com wrote:

Hi Ed,

I've added support for extensions and reporting. I've also moved to symbols for the JSON parsing to make it easier to extend and keep in a Ruby-esque tone.

I'm a tad new to this pull malarkey, so I hope I've done this right. Would you confirm, please?

Thanks,

Jason


Reply to this email directly or view it on GitHub.

@kitebuggy
Copy link
Author

Thanks Ed!

Sent from my iPhone

On 13 Dec 2015, at 21:39, Ed Jones notifications@github.com wrote:

Ace! Will look ASAP :-)

On 13 Dec 2015, at 21:30, Jason notifications@github.com wrote:

Hi Ed,

I've added support for extensions and reporting. I've also moved to symbols for the JSON parsing to make it easier to extend and keep in a Ruby-esque tone.

I'm a tad new to this pull malarkey, so I hope I've done this right. Would you confirm, please?

Thanks,

Jason


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.

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