Skip to content

Session: added getFlashSection()#5

Closed
dg wants to merge 1 commit into
nette:masterfrom
dg:pull-tab
Closed

Session: added getFlashSection()#5
dg wants to merge 1 commit into
nette:masterfrom
dg:pull-tab

Conversation

@dg

@dg dg commented Jul 1, 2014

Copy link
Copy Markdown
Member

No description provided.

Comment thread src/Http/Session.php Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Random::generate() can fail?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@Majkl578

Majkl578 commented Jul 3, 2014

Copy link
Copy Markdown
Contributor

Not sure why, but I don't like this. It's completely out of Session's scope. Also this is only a nette\application thing. Not sure it should be hardcoded into Session this way.

@mishak87

mishak87 commented Jul 3, 2014

Copy link
Copy Markdown

This whole code should be object dependent on Session new FlashSessionStorage($this->getSession()) and in nette/application.

@dg

dg commented Jul 3, 2014

Copy link
Copy Markdown
Member Author

I have two implementations. This is one and the second one uses new class Nette\Http\FlashSession. I'm not sure which way to go. But it is definitely not only nette/application thing, because it solves common problem with session ID identifier.

@mishak87

mishak87 commented Jul 3, 2014

Copy link
Copy Markdown

@dg Sorry I had in mind Tab and after searching just found out you changed the name back from Tab to Flash. nette/application#4 (comment)

Where is the second one?

@dg

dg commented Jul 3, 2014

Copy link
Copy Markdown
Member Author

The second one is only in my local branch.

„Tab“ idea was completely wrong. I realized that the important is that it exists only during the redirect.

@mishak87

mishak87 commented Jul 3, 2014

Copy link
Copy Markdown

Instead of explaining further I have created two pulls it should be clear from them what I meant.

NOTE: Separating the session namespace from id is great idea the class just extends on that.

@dg dg force-pushed the master branch 4 times, most recently from 287cae9 to f55bed2 Compare September 5, 2014 21:34
@dg dg force-pushed the master branch 9 times, most recently from 0bb4336 to 96b498c Compare December 27, 2014 07:22
@dg dg closed this Jan 25, 2015
@dg dg deleted the pull-tab branch January 25, 2015 19:11
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.

4 participants