Skip to content
This repository has been archived by the owner on Feb 28, 2020. It is now read-only.

Bring tag behavior in line with builtin messages #27

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nthall
Copy link

@nthall nthall commented Feb 19, 2016

This PR fixes issue #21 -- it adds a level_tag property to the message model, and adds a custom message manager with a method that automatically adds any applicable level_tag to any user-defined tags. Result is that tag behavior now matches builtin messages. I didn't add any new tests for this functionality; I just changed the existing tag-related assertions to match the new expected behavior. I'm happy to add new tests if you think it's necessary.

@nthall
Copy link
Author

nthall commented Jul 14, 2016

Hi, just checking in - if there's something I need to fix to get this accepted, I will happily fix it.

@reinout
Copy link

reinout commented Sep 5, 2016

(Helping a colleague with his pull request, so I'm browsing the others)

A quick win would be if you'd pull in master so that you get the tox.ini + .travis.yml for the automatic tests, I guess.

@nthall
Copy link
Author

nthall commented Oct 31, 2016

Hi, I have some time and attention for this again. I'm sorry for being dense but I don't know how to change this to get Travis to run. Do I need to close and do something differently in a new pull request? Is there something else I need to fix?

@reinout
Copy link

reinout commented Oct 31, 2016

Well, it all depends on the project maintainers to do something. What you could try in your branch:

$ git fetch
$ git merge master
# see if it all works still
$ git push

That ought to put the ".travis.yml" from master into your branch, which ought to automatically configure travis to actually run. So you'd only have to do the merge from master.

(Having the automated test makes it easier to get someone to merge it).

@nthall
Copy link
Author

nthall commented Oct 31, 2016

Thank you @reinout ! And now I have passing checks but an ugly commit history - is it safe to rebase this and force push or should I just leave it?

@reinout
Copy link

reinout commented Oct 31, 2016

I'm not a hero with git, so rebasing and so always feels dangerous to me (probably unfounded). You'll have to ask one of the maintainers ( @palazzem?), I don't maintain this project, I was just passing through :-)

@nthall
Copy link
Author

nthall commented Dec 14, 2016

OK, I've rebased on top of master and cleaned up the commit history. Would love to resolve this at last :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants