Repo name/slug/etc. in repository functions about sharpbucket HOT 5 CLOSED

Incidentally, looking back at #73, it seems odd that I need to pass in a slug to get a RepositoryResource, but PostRepository doesn't use this slug at all -- it (correctly) uses the slug in the Repository that's passed in to that method.

from sharpbucket.

I'm totally agree that methods that take a string parameter named repository is not really clear for users

Now how should we rename that parameters and how should we manage it ? I'm less sure to be totally agree with what you exposed.

I think that most of the time consumers know the name of their repositories, but less will know the slug, and for the UUID it may even be impossible to know it... Like when we use PostRepository to create a new repository. It's natural to only know the name at that moment

So, I think it's important to continue to provide an auto 'slugification' in some points of the API (especially public ones). An in that cases I suggest to rename the repositoryparameter as repoSlugOrName
And for internal methods (or maybe some public ones, we should check them one by one to validate the use cases), where we expect 'slugification' to have be already performed, rename the repositoryparameter as repoSlug

I'm not in favour of introducing OrUuid in the parameters names because:

• Even in Bitbucket documentation it doesn't appear in the parameters names, just in the documentation
• There is really few scenario where we are interesting in using the UUID and not the slug (renaming is the only one that came in my mind), so it rare enough to be just in the documentation and not in the name like in Bitbucket documentation

This doesn't prevents us to support UUID in that parameters, but I think it is preferable to support them like Bitbucket support them, has a UUID string surrounded by curly-braces send in the repoSlugOrName parameter. In other words, detect that type of string and consider it's already a 'slugified' string

Our ParseSlug should then looks to something like

        private string ParseSlug(string repoSlugOrName)
        {
            if (Guid.TryParseExact(repoSlugOrName, "B", out _))
                return repoSlugOrName;

            var slugRegex = new Regex(@"[^a-zA-Z\.\-_0-9]+");
            return slugRegex.Replace(repoSlugOrName, "-").ToLowerInvariant();
        }

from sharpbucket.

I agree with most of what you say. A few observations:

1. Personally, I know the slugs of my repos better than I know their names, since the slugs show up when I'm cloning a repo and get turned into the local directory name.
2. If you want to allow the user to pass in a slug or a name (and slugify automatically), then I think it's also reasonable to allow UUIDs to be passed in with or without braces, and add the braces in the library if needed (see my post above).
3. I'm fine with not putting OrUuid in the parameter name, as long as we can point out in the Intellisense that it can be a UUID.

Should I start a PR so we have something more concrete to discuss?

from sharpbucket.

For your second point, if we try to support various formats of UUID strings, it may conflict with our unit tests were we create repositories which names are UUIDs without braces (guids formatted with the "N" format, which means 32 digits without braces nor hyphens), which are also valid slug, but definitively not there Uuids

Should we reworks our unit tests to avoid to create that case ?
Or should we stick with the only Uuid string format that Bitbucket support and avoid to add too much additional features in that client ?

@MitjaBezensek can you help us to decide, since we are just two in that discussion with two different point of view ?

Except that, you can start a PR since there is probably more discussions to come when we will have the concrete rename locations to decide if such layer should slugify or if it should be done before (I'm not sure that current state is totally the good one without looking at all the cases)

from sharpbucket.

Good point about the unit tests.

My preference would be for the library to accept UUIDs in either D format (hyphens, no braces) or B format (hyphens and braces). This would let users use what are probably the two most common forms and wouldn't affect the unit tests.

My thinking is that since the library automatically lowercases when slugifying, so that the user doesn't need to remember that slugs have to be lowercase, it should also add braces if needed so that the user doesn't need to remember that either.

I'll start doing some work on a PR.

from sharpbucket.