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

Replace match via get with get on routes #7987

Merged
merged 1 commit into from Jan 15, 2015

Conversation

5 participants
@cirosantilli
Contributor

cirosantilli commented Oct 7, 2014

Shorter, matches the style of the rest of the routes, and match raises red flags for me because of the possibility of routing multiple methods to a single action with it: http://guides.rubyonrails.org/routing.html#http-verb-constraints

@TeatroIO

This comment has been minimized.

TeatroIO commented Oct 7, 2014

I've prepared a stage. Click to open.

@jvanbaarsen

This comment has been minimized.

Contributor

jvanbaarsen commented Dec 14, 2014

@cirosantilli Can you please make this mergeable again?

@cirosantilli cirosantilli force-pushed the cirosantilli:get-instead-match branch from 10d5e08 to dec5215 Dec 29, 2014

match "/u/:username" => "users#show", as: :user,
constraints: {username: /(?:[^.]|\.(?!atom$))+/, format: /atom/}, via: :get
get '/u/:username' => 'users#show', as: :user,
constraints: {username: /(?:[^.]|\.(?!atom$))+/, format: /atom/}

This comment has been minimized.

@houndci-bot

houndci-bot Dec 29, 2014

Space inside { missing.
Space inside } missing.

@cirosantilli cirosantilli changed the title from Replace match via get with get on routes to [WIP] Replace match via get with get on routes Dec 29, 2014

@cirosantilli cirosantilli force-pushed the cirosantilli:get-instead-match branch from dec5215 to 29ed462 Dec 29, 2014

@cirosantilli cirosantilli changed the title from [WIP] Replace match via get with get on routes to Replace match via get with get on routes Dec 29, 2014

@cirosantilli

This comment has been minimized.

Contributor

cirosantilli commented Dec 29, 2014

@jvanbaarsen updated.

@jvanbaarsen jvanbaarsen added this to the 7.7 milestone Jan 4, 2015

@dzaporozhets dzaporozhets modified the milestones: 7.7, 7.8 Jan 13, 2015

dzaporozhets added a commit that referenced this pull request Jan 15, 2015

Merge pull request #7987 from cirosantilli/get-instead-match
Replace match via get with get on routes

@dzaporozhets dzaporozhets merged commit d4a8471 into gitlabhq:master Jan 15, 2015

2 checks passed

default The build passed on Semaphore.
Details
semaphoreci The build passed on Semaphore.
Details

@cirosantilli cirosantilli deleted the cirosantilli:get-instead-match branch Jan 16, 2015

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