used werkzeug.routing for using regex in routes to match string ending with only git
Rather than relying on a regex, did you try something like:
@APP.route('/<repo:repo>.git')
Trailing spaces :)
Oh, and the trailing slash in the URL is probably not needed then :)
Would you also be interested in adding unit-tests for this change?
@pingou
I used regex as it will be better and fast for matching repo name ending with .git.
I can use a different approach if you want. Your Suggestions are welcome.
Thanks
anybody is free to disagree but, i would prefer to use flask.redirect than redirect especially when we have to import for one case (also, we have already used flask.redirect in this file)
flask.redirect
redirect
+1 on this
Would it really be faster than the approach I suggested? (which isn't tested though, but more what I had in mind for this feature).
I used regex as it will be better and fast for matching repo name ending with .git. Would it really be faster than the approach I suggested? (which isn't tested though, but more what I had in mind for this feature).
Should i try it using the approach suggested by you?
@vivekanand1101
Thanks for the suggestion, I will definitely change it to flask.redirect.
If it works and isn't so slow I think it would make the code simpler
Should i try it using the approach suggested by you? If it works and isn't so slow I think it would make the code simpler
@pingou ok then , i will go with your approach
1 new commit added
That will work for project, don't we want to support forks as well?
I am adding for the fork part too :)
So are we finally going with this now ?
Looks simpler to me, doesn't it? :)
Sure, its too simple :) I learnt a new lesson today. Thanks to you!
@pingou just one question Should i create a different route for the fork part too ?
@abhishekg785 we can do this in the same view: remove .git and redirect
.git
I did not understand what do you mean by removing .git.
@abhishekg785, check flask.request.url
flask.request.url
yes we can use flask.request.url for this but it was decided that we have to do by creating a route for this
What i am trying to say is, here you have created two view functions with one route each. We can create just one view with two routes (the other one being the fork one).
At the beginning i did it in the same view without using request.url bcoz the route accepting repo name will accept all strings after '/' so, we can simply check if a string contains .git then split the string on dot and pass the repo name without .git . So it even did not require "request.url "
But it was decided to use route for the purpose.
oh! my mistake :)
Hey, i can simply check for username If it is not none then simply redirect for the fork case.
No need for request.url. What do you think?
:thumbsup: for me
I think I would just do the normal: flask.redirect(flask.url_for('view_repo', repo=repo, username=username))
flask.redirect(flask.url_for('view_repo', repo=repo, username=username))
ok, @pingou wins again, this one is better :/
I think we should remove the two empty lines added here
Ok, one last comment but it looks good.
Two things outside the code though:
I think squashing the commits will be a good choice.
Let's squash and rebase then :)
In the original code there were three empty lines after the comment '# pylint: disable=E1101',
so should i remove the two lines as you mentioned ?
Well, looking at this it seems there were only 2 empty lines here
my bad:) , changing it back to the original 2 line empty lines and then squashing the commits
rebased
Hm it seems it detected a rebase but apparently it still needs one
2 new commits added
Commit messages need love in this :smile:
Looks like the rebase is done but the commits are odd :s
@abhishekg785 since you will be removing the spaces, can you please change the commit message as well ? something like: Redirect the urls for git repo to the project index page
Redirect the urls for git repo to the project index page
i tried to open the file in vim and other text editors ,but there is no space in my case. I even pulled the original code ,still the condition is same.
just wierd :)
Ok so I'll merge this manually
Commit 40daa05d fixes this pull-request
Pull-Request has been merged by abhishekg785@gmail.com
used werkzeug.routing for using regex in routes to match string ending with only git