handle trailing slash with engines (test case for #7842) #8115

Merged
merged 2 commits into from Nov 8, 2012

Conversation

Projects
None yet
4 participants
@senny
Member

senny commented Nov 4, 2012

This PR adds a test case to make sure that we don't get any regressions on #7842. Also I removed obsolete code from the _generate_prefix method.

The issue described in #7842 has been fixed on master but is still present on 3-2-stable. The attached test case will fail on 3-2-stable and passes on master.

@senny

This comment has been minimized.

Show comment Hide comment
@senny

senny Nov 4, 2012

Member

I did not create a CHANGELOG entry, since I did not modify any existing functionality. Also I split the commits because they are not related to each other and adress separate concerns.

@steveklabnik @rafaelfranca can you take a look? let me know if I need to change things up.

Member

senny commented Nov 4, 2012

I did not create a CHANGELOG entry, since I did not modify any existing functionality. Also I split the commits because they are not related to each other and adress separate concerns.

@steveklabnik @rafaelfranca can you take a look? let me know if I need to change things up.

@steveklabnik

This comment has been minimized.

Show comment Hide comment
@steveklabnik

steveklabnik Nov 4, 2012

Member

Seems legit to me.

Member

steveklabnik commented Nov 4, 2012

Seems legit to me.

@schneems

This comment has been minimized.

Show comment Hide comment
@schneems

schneems Nov 6, 2012

Member

I say 👍 for testing this regression, though would want this squashed into 1 commit. Anything to add @steveklabnik? @senny since the previous behavior is a bug, would you be willing to submit your fix against the 3.2 branch?

Member

schneems commented Nov 6, 2012

I say 👍 for testing this regression, though would want this squashed into 1 commit. Anything to add @steveklabnik? @senny since the previous behavior is a bug, would you be willing to submit your fix against the 3.2 branch?

@senny

This comment has been minimized.

Show comment Hide comment
@senny

senny Nov 6, 2012

Member

sure, I already did the fix for 3.2 because I thought it was still present in master.

Member

senny commented Nov 6, 2012

sure, I already did the fix for 3.2 because I thought it was still present in master.

@senny

This comment has been minimized.

Show comment Hide comment
@senny

senny Nov 7, 2012

Member

@schneems I split the two commits, because the test-case can be backported and refactoring not.

@rafaelfranca what do you think?

Member

senny commented Nov 7, 2012

@schneems I split the two commits, because the test-case can be backported and refactoring not.

@rafaelfranca what do you think?

rafaelfranca added a commit that referenced this pull request Nov 8, 2012

Merge pull request #8115 from senny/7842_handle_trailing_slash_with_e…
…ngines

handle trailing slash with engines (test case for #7842)

@rafaelfranca rafaelfranca merged commit 3148ed9 into rails:master Nov 8, 2012

@schneems

This comment has been minimized.

Show comment Hide comment
@schneems

schneems Nov 8, 2012

Member

Thanks for your work @senny.

Member

schneems commented Nov 8, 2012

Thanks for your work @senny.

senny added a commit to senny/rails that referenced this pull request Nov 8, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment