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

allow periods (.) in project names #142

Closed
ghost opened this issue Aug 1, 2012 · 6 comments
Closed

allow periods (.) in project names #142

ghost opened this issue Aug 1, 2012 · 6 comments

Comments

@ghost
Copy link

ghost commented Aug 1, 2012

It's not possible to use freely-defined project-names as they have to match the folder name of the build, at the moment (meaning only characters the file system allows can be used). Although it would be nice to be able to do so, my bigger concern was to use periods in project names, which should be possible with filename-compliant names... To be able to do so, I had to change the routing as follows:

--- orangenschale.orig/config/routes.rb 2012-07-12 05:14:44.000000000 -0500
+++ orangenschale/config/routes.rb  2012-07-15 11:15:42.071137700 -0500
@@ -10,10 +10,10 @@

   resources :projects, :only => 'index'

-  get '/projects/:project_name' => 'projects#show', :as => :project
-  post '/projects/:project_name/builds' => 'projects#force', :as => :project_force
-  put '/projects/:project_name/builds/:build_number/cancel' => 'builds#cancel', :as => :build_cancel
+  get '/projects/:project_name' => 'projects#show', :constraints => {:project_name => /[^\/]+/}, :format => false, :as => :project
+  post '/projects/:project_name/builds' => 'projects#force', :constraints => {:project_name => /[^\/]+/}, :as => :project_force
+  put '/projects/:project_name/builds/:build_number/cancel' => 'builds#cancel', :constraints => {:project_name => /[^\/]+/}, :as => :build_cancel

-  get '/projects/:project_name/builds/:build_number' => 'builds#show', :as => :project_build
-  get '/projects/:project_name/builds/:build_number/artefacts/:path' => 'builds#artefact', :constraints => {:path => /.*/}, :as => :project_build_artefact
+  get '/projects/:project_name/builds/:build_number' => 'builds#show', :constraints => {:project_name => /[^\/]+/}, :as => :project_build
+  get '/projects/:project_name/builds/:build_number/artefacts/:path' => 'builds#artefact', :constraints => {:path => /.*/,:project_name => /[^\/]+/}, :as => :project_build_artefact
 end
@srushti
Copy link
Owner

srushti commented Aug 1, 2012

The problem is the substitution strategy. If we replace unusable characters with an underscore or something, then you can't later add another project which really has underscores there. So, for now I'm leaning towards leaving this alone. Let me think about it for a bit.

@ghost
Copy link
Author

ghost commented Aug 1, 2012

Well, I understand, however, the patch is not about allowing any kind of project name, it's simple deactivating rails' behaviour to 'format'. If right now you'd add a project with a period in it, rails will simply display an error page, as it tries to match the 'suffix' to a format.

In my example, I have projects running builds on different emulators to test for portability, so a project has a name like:
my_project_on_OpenBSD-4.4 - which would fail.

@srushti
Copy link
Owner

srushti commented Aug 1, 2012

Ah, ok. I understand now.

That, of course, causes another problem. If you have project called 'foo' your browser url would become '/projects/foo', and we also have '/projects/foo.png' which returns an image with the current status of the build to use in documentation pages. If we ignore dots in those urls that might stop the dot in '.png' also from working.

Let me think about this a bit more (unless you have a different suggestion).

@ghost
Copy link
Author

ghost commented Aug 1, 2012

Interesting, I didn't even know that...
Well, the :constraints => {:project_name => /[^\/]+/} could be changed to match everything except for .png suffixes, etc..

srushti added a commit that referenced this issue Aug 11, 2012
this was done by adding a constraint on project_name which would accept anything and then adding explicit routes for the png (& explicit html) format routes.
@srushti
Copy link
Owner

srushti commented Aug 11, 2012

Fixed. Look at the commit above if you're curious. Basically, I figured out what approach travis-ci took to solve the problem.

@srushti srushti closed this as completed Aug 11, 2012
@ghost
Copy link
Author

ghost commented Aug 13, 2012

Thanks, that's definitely cleaner than my version!

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

No branches or pull requests

1 participant