#866 Add support for pseudo-namespace in pagure
Merged by pingou. Opened by pingou.
namespace  into  master

Download 866.patch
no initial comment

Pseudo-namespace basically means we do not support namespaces per say but are allowing repositories to have a single slash ('/') in their names.

This will allow projects like: fedora/kernel or rpms/guake which we will want to use if we
deploy pagure as a front-end for pkgs.fedoraproject.org.

Pull-Request has been rebased

Pull-Request has been rebased

Pull-Request has been rebased

Pull-Request has been updated

s/you can/you care/

Does this mean that forks are now also in the GIT_FOLDER instead of a separate one?

yup, they are moved

So in the part before the /, I can't start the name with a -, whereas I can do that in the second part?
So this is valid: m-/-

This is missing the repo:repo

Perhaps you can make sure it's impossible to create a project with a name with a slash where there are exactly 40 characters after the slash?
People shouldn't hit this check anytime in normal use, but it would make sure that if they do they can't get in weird situations.

Please note that this and the one below are missing the repo: part in

Perhaps:
regex = '[^/]*/?[^/]{,39}'

What happens if I create a repo that I name "forks/something"?

Also, what if I create a project named whatever/c ?

I would love to see some more negative tests: what happens if I create a project with various types of names that might break things? For examples, see some of my comments.

So you are breaking more urls than just the //, but also //. Might want to thoroughly document that.

Also, you probably need to create documentation about what to do while upgrading, preferably as a file in the repo that draws a lot of attention.
Especially in this case, when you need to make sure you migrate the forks directory.

hm, we just need to avoid the 40 chars, 39 or 41 are right no?

If so then I think we should check this and fix it in python rather than the regex.

Fixed all those

I'll adjust the python logic for this

Any thought where I can document it?

Ok, I fixed this where I fixed the regex for project name, since it now requires
at least 2 characters after the '/' :)

Sure, agreed. I just thought that limiting to 39 characters seems reasonable to me, and the easiest fix.

In the upgrading document? :-)

Fair enough of a fix :-)

Sounds good

Pull-Request has been rebased

Pull-Request has been updated

Pull-Request has been updated

I am not entirely sure that breaking the string up with \ between lines makes it readable, as I had to read multiple times to see the correct one.
Perhaps you use
PROJECT_NAME_REGEX = ('^..../'
'?[a....%')?

There is a linebreak between ./' and '? in my example.

Perhaps add a comment as to why there's this limitation?

I am not sure whether we should show the full blacklist... I would suggest just saying "This group name has been blacklisted, please choose another one".

I'm not sure using brackets is more readable tbh :(

So I went for
PROJECT_NAME_REGEX = \
'^[a-zA-z0-9_][a-zA-Z0-9-]*/?[a-zA-z0-9][a-zA-Z0-9-_]+$'

This is kind of an odd sentence :).

Pull-Request has been updated

:ok_woman: Looks good to me. Please do make sure to merge an upgrading document before release, but this PR itself looks good to me.

Thanks for the review @puiterwijk! :)

Pull-Request has been merged by pingou

Metadata