Skip to content
This repository was archived by the owner on Jan 8, 2020. It is now read-only.

First contrib. and commit. #9

Closed
wants to merge 8 commits into from
Closed

Conversation

rettal
Copy link
Contributor

@rettal rettal commented Sep 20, 2010

Hi,

Ive added new exception support to \Zend\Dom.

This is my very first commit so i may have got things wrong :). I am aware I probably should have created a branch but I am just getting used to Git (git Bash) as I have not used it before so for this instance I have commited to my master branch so I hope this is OK.

The changes I made were very small, I just created a new RuntimeException class, updated Query.php to use this class and updated the comments. Ralph Schindler did say that I would also need to create an InvalidArgument class but couldnt quite see where this would easily go.

If anything else needs to be done to this class or if I have done anything wrong please let me know.

Thanks

@ralphschindler
Copy link
Member

Almost, but not quite :)

Here are some suggestions. First, you'll need to create a new branch to work out of based on milestones/exceptions, perhaps you might call it milestones/exceptions-dom, or something to that effect. That branch will contain your Dom based work.

As for the code, the main Exception should be an interface. You should also create an Exception/ namespace and put the RuntimeException in that namespace. The RuntimeException should extend the SPL \RuntimeException class and implement the \Zend\Dom\Exception interface.

That should do it, you can catch me in #zftalk.dev to discuss how to get this in a place that can be merged in.

@rettal
Copy link
Contributor Author

rettal commented Sep 21, 2010

Thanks for the reply.

I have made the ammendments you suggested. Sorry to say I only scan read the post you linked to the other day on zfdev so im gonna read that properly tonight :)

I am using Git Bash for windows with Github as my remote repo. I currently have all the changes in a separate branch of my master zf2 branch (forked from zendFramework).

Should I commit my changes to milestones/exceptions branch on Github? If so how do I commit to a branch on Github using Git Bash, cant seem to get do it?

Thanks for all your help, Im sure I am nearly there :)

Also, I followed the same approach adopted with the Zend/Acl Exception changes, which I think I read you mentioned was OK to do so i a blog post?

ralphschindler referenced this pull request in weierophinney/zendframework Jul 26, 2011
…lection

Merging Zend\Http request header collection
weierophinney pushed a commit that referenced this pull request Dec 16, 2011
fabwu pushed a commit to fabwu/zf2 that referenced this pull request Oct 11, 2014
gianarb pushed a commit to zendframework/zend-memory that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-paginator that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-http that referenced this pull request May 15, 2015
…/http-foundation-header-collection

Merging Zend\Http request header collection
gianarb pushed a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-feed that referenced this pull request May 15, 2015
weierophinney pushed a commit that referenced this pull request Dec 20, 2016
- [#9](zendframework/zend-form#9) Correct typo in aria
  attributes' names

  There should not be a dash before “by”:

  - http://www.w3.org/TR/wai-aria/states_and_properties#aria-labelledby
  - http://www.w3.org/TR/wai-aria/states_and_properties#aria-describedby

  This fixes a typo I introduced in #5989.

- [#12](zendframework/zend-form#12) Deprecated
  AllowEmpty and ContinueIfEmpty annotations
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants