Skip to content
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

Notwritten route pass to NotFound handler #246

Merged
merged 2 commits into from
Apr 25, 2014
Merged

Notwritten route pass to NotFound handler #246

merged 2 commits into from
Apr 25, 2014

Conversation

sevkin
Copy link
Contributor

@sevkin sevkin commented Apr 25, 2014

m.Get("/:page", func(p martini.Params, r render.Render) {
    if p["page"] == "im-exists" {
        r.HTML(200, "page", "all ok")
    }
    // else (another matched but not exists) - do nothing, pass me to NotFound
}

it`s a version 2 of #229

erizocosmico pushed a commit that referenced this pull request Apr 25, 2014
Notwritten route pass to NotFound handler
@erizocosmico erizocosmico merged commit 7d1c704 into go-martini:master Apr 25, 2014
@sevkin
Copy link
Contributor Author

sevkin commented Apr 25, 2014

@sgp I`m think is not a problem. application may explicit return 204

func main() {
    m := martini.Classic()
    m.Get("/", func(rw http.ResponseWriter) {
        // do something without content
        rw.WriteHeader(204)
    })
    m.Run()
}

as expected 204 without content

$ curl -i http://localhost:3000/
HTTP/1.1 204 No Content
Date: Fri, 25 Apr 2014 16:26:45 GMT
Content-Length: 0
Content-Type: text/plain; charset=utf-8

@sgp
Copy link

sgp commented Apr 25, 2014

This is true; although this needs to be documented as a necessary change to existing code. We had many cases in our code where we used the default behavior (i.e. nothing written == 200 OK).

codegangsta added a commit that referenced this pull request Apr 25, 2014
@codegangsta
Copy link
Contributor

I just reverted this change. I'm happy to have this in as long as we document it's usage and notify current martini users of breaking changes on the mailing list before we land it.

@sgp
Copy link

sgp commented Apr 25, 2014

I think that this kind of change would be better suited by having an explicit method call or return value that can grant this sort of behavior. Returning 200 OK by default (i.e. assuming handled, not "not found") is better default behavior IMHO.

@codegangsta
Copy link
Contributor

Thanks for the input Scott!

On Fri, Apr 25, 2014 at 11:55 AM, Scott Parkerson
[email protected]:

I think that this kind of change would be better suited by having an
explicit method call or return value that can grant this sort of behavior.
Returning 200 OK by default (i.e. assuming handled, not "not found") is
better default behavior IMHO.


Reply to this email directly or view it on GitHubhttps://github.com//pull/246#issuecomment-41427415
.

@sgp
Copy link

sgp commented Apr 25, 2014

No problem. We've been enjoying Martini here. :)

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