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

Fixing single character tags #82

Merged
merged 2 commits into from
Jun 29, 2020
Merged

Fixing single character tags #82

merged 2 commits into from
Jun 29, 2020

Conversation

mcrot
Copy link
Contributor

@mcrot mcrot commented Oct 15, 2019

I've just opened issue #81, because I cannot use single character tags with the current version
of tagulous. Here is a pull request which might fix the issue.

First I've added some tests to show the problems I have. Second, I've replaced the function
utils.split_tree_name(name) with simpler code.

Do I miss something or can it just be implemented by using the split() method of strings, as I did?
One drawback of my solution is that you cannot use a NULL character in tags anymore,
but that shouldn't be problem - although there also would be a solution for that.

What do you think? The tests are passing, also the new ones I've added.

@coveralls
Copy link

coveralls commented Oct 15, 2019

Coverage Status

Coverage decreased (-0.2%) to 95.271% when pulling b132e9a on mcrot:develop into 62d6528 on radiac:develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 95.247% when pulling b132e9a on mcrot:develop into 62d6528 on radiac:develop.

@mcrot
Copy link
Contributor Author

mcrot commented Oct 16, 2019

Hm - how can I improve the coverage? It seems like that most uncovered code is related to
old Django versions.

@mcrot mcrot requested a review from radiac February 10, 2020 11:50
@mcrot
Copy link
Contributor Author

mcrot commented Feb 10, 2020

@radiac , I would like to ask you whether you could review these changes and help me to increase the test coverage a bit.

I think this is a useful change, but I don't know how to increase the test coverage. It seems like all lines not covered are not covered because they are only needed when using old Django versions or South.

@mcrot mcrot mentioned this pull request Feb 10, 2020
@radiac radiac merged commit cde37c6 into radiac:develop Jun 29, 2020
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.

None yet

3 participants