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

NFe Access Key Validator #138

Closed
wants to merge 10 commits into from
Closed

NFe Access Key Validator #138

wants to merge 10 commits into from

Conversation

andreyvital
Copy link
Contributor

No description provided.

@nickl-
Copy link
Member

nickl- commented Nov 28, 2013

@andreyknupp Awesome! =)

Please also update documentation if you don't mind.

@nickl-
Copy link
Member

nickl- commented Nov 29, 2013

Oops something went wrong there and now includes commits from @alganet and @henriquemoody =(

@andreyknupp thank you for the docs!!! It all seems to be there but slightly broken.

If you would be so kind as to resubmit this PR from a fresh fork with only your changes it will simplify my tasks, otherwise I will have to investigate first.

@nickl-
Copy link
Member

nickl- commented Jan 1, 2014

@andreyknupp I think you can fix it without going through all that effort.

Try this: (I assume that your repo is origin if you cloned from the fork.)

$ git remote add upstream https://github.com/Respect/Validation.git
$ git fetch upstream 
$ git rebase upstream/develop
$ git push origin develop

Your commits will now be on top of the latest on HEAD and if you want to "roll back" you can reset HEAD~2 to go back for the 2 commits, make the fixes you want and do new commits. This will get you out of sync with your fork as it would not have done the reset and expects to go forward only but this is not a disaster if you add --force to your push it will accept the out of sync changes.

Once you're done come look back over here, as the PR would've updated itself... like magic Ooooohhh =)

Tell us how it goes...

@augustohp
Copy link
Member

Sweet! This indeed needs a rebase against master. If needed, I can do it (don't do it now because my internet connection sucks good time) just let me know.

@andreyknupp a question: an "nfe access key" is what (I am not familiar with NFe)? My question is target specially if the name for the rule nfe is appropriate and if nfeAccessKey is a better name.

@augustohp
Copy link
Member

@andreyknupp I knew what NFe means, I just don't know the workings and what the access key is for, that was my question intended for. 😜

It seem like a solid PR to me 🍻! Kudos for the tests.
Could you change the name (if you still think it is relevant) and re-do the PR against master?! If you need any help, please let me know.

@augustohp augustohp self-assigned this Feb 16, 2014
@augustohp
Copy link
Member

Hey @andreyknupp I see, sorry to bother you with that after so much time. There are easier alternatives, you can checkout the pull request locally on you new fork and work on top of that.

By the way, I've done that to you so you don't have to after al; the work and patience you had with us already. 😛
I've done the rebase (against master) and the rename on #147, please let me know of any problems or issues you may find with the pull request, I will wait for you feedback to merge the PR.

PS: I am closing this in favor of #147.

@augustohp augustohp closed this Feb 16, 2014
@augustohp
Copy link
Member

@andreyknupp Awesome! I merged a different pull request because it still supports 5.3 I kept you as original author since you've done all the work! 🍻

Thanks a lot for this new validator!

@augustohp augustohp modified the milestones: 0.6.0, 0.6.1 Feb 20, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants