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

Adding cookie route. Going to open PR for comments. #2772

Closed
wants to merge 60 commits into from
Closed

Adding cookie route. Going to open PR for comments. #2772

wants to merge 60 commits into from

Conversation

starJammer
Copy link

Creating a cookie route that will route you according to cookies you have set or that you don't have set.

I was inspired by the approach taken in www.yiibu.com

To understand you might have to delete the cookies this site sets several times.

They route you to a 'setup' page if you don't have a certain cookie. Otherwise, they let you pass on through to the normal page you want.

marc-mabe and others added 30 commits August 16, 2012 23:24
…viously string(5.00) did not equal int(5) after it was parsed
[ZF2-550]   Removed type check so that string("5.00") is recognized as equal to int(5) to validate it is a float
@starJammer
Copy link
Author

I forgot to mention in the PR that i'd like opinions on this approach or if there is a better way of doing it.

@ghost ghost assigned DASPRiD Oct 30, 2012
@DASPRiD
Copy link
Member

DASPRiD commented Oct 30, 2012

First of all, you should review PSR-1 (https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-1-basic-coding-standard.md) and PSR-2 (https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md), so your code complies with the coding standard.

On a first look, there are also some architectural things, which I will comment on within the next 24 hours.

@starJammer
Copy link
Author

Thanks DASPRID. I'll go back and change my code to comply. Just wanted someone like yourself to comment with architectural advice to see if this is even useful or if there are other options for accomplishing the same thing.

Thanks for you willingness to help.

@starJammer
Copy link
Author

Hey DASPRiD,
I just wanted to see if you have had time to think about the architectural impact this change might have. I haven't updated my code yet because I'd rather you tell me if there is a better option instead of spending time on this when it might not be accepted. Should I open up an issue on the Zend issue tracker or do anything to get more opinions?

@DASPRiD
Copy link
Member

DASPRiD commented Nov 20, 2012

Hi @starJammer – wonder if you could ping me in IRC (#zftalk.dev on freenode) to discuss your implementation.

@starJammer
Copy link
Author

Hey DASPRiD,

Sure. What times are you available. This week won't be good for me until maybe Friday night. Thanksgiving is coming up in the usa and I'll be enjoying festivities on Thurs.

@weierophinney
Copy link
Member

@starJammer and @DASPRiD -- have you had a chance to discuss this yet? Would like to see some progress. :)

@starJammer
Copy link
Author

@DASPRiD I'll be online during the day tomorrow if you'd like. I'll be on the irc channel tomorrow at 12pm EST time.

Jerry Saravia added 3 commits December 19, 2012 01:14
Conflicts:
	library/Zend/Code/Scanner/ClassScanner.php
	library/Zend/Code/Scanner/PropertyScanner.php
@starJammer starJammer closed this Dec 19, 2012
@starJammer starJammer reopened this Dec 19, 2012
@starJammer
Copy link
Author

Not sure on what I did here. I probably should not have updated my cookie branch with the latest changes from master.

@weierophinney
Copy link
Member

@DASPRiD are the new changes @starJammer introduced based on your IRC discussion? is this ready to merge?

@starJammer
Copy link
Author

No we haven't met yet @weierophinney. I ran my code through a prs tool and changed some code based on some bugs I encountered.

@starJammer starJammer closed this Jan 4, 2013
@starJammer
Copy link
Author

Closed after discussion with DASPRiD. Conclusion :
If functionality were to be added like this it would fit in better as a listener. Most likely it would listen to the route or preroute event and act accordingly based on cookie values in the request.

At the moment though, this functionality isn't something that is necessary in the core.

@starJammer starJammer deleted the CookieRoute branch January 4, 2013 13:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants