There is one more idea/proposal for the create_test.py improving and the way we invoke topology objects.
Instead of writing tests like this:
from lib389.topologies import topology_m2 def test_something(t): """Test case""" topology_m2.ms["master1"].stop() topology_m2.ms["master1"].start()
We can write like this:
from lib389.topologies import topology_m2 as t # We can use it in our code, optionally m1 = t.ms["master1"] def test_something(t): """Test case""" t.ms["master1"].stop() m1.start()
For that, I can refactor create_test.py a bit, so it will make all initial actions for you. What do you think?
I don't like the short variable names like "t" as it gives no context to what is happening. I'm guilty sometimes of doing things like this, but I don't think we should encourage it widely.
A compromise would be "import topology_m2 as topo" or something like that, which is shorter but at least still indicative or a partial word.
Ultimately, is this really a big problem for us? What will this solve for us? How does it help us?
Replying to [comment:1 firstyear]:
I don't like the short variable names like "t" as it gives no context to what is happening. I'm guilty sometimes of doing things like this, but I don't think we should encourage it widely. A compromise would be "import topology_m2 as topo" or something like that, which is shorter but at least still indicative or a partial word. I agree with you, I don't like and don't use such variables too. Topology is a bit another thing. It has its "main" part of the name i.e. "master", "consumer etc. So in my opinion, {{{ t.ms["master1"] }}} is still very readable. Though I do fully agree to the "'''topo'''" too, it is still better than "'''topology_m1h1c1'''". Ultimately, is this really a big problem for us? What will this solve for us? How does it help us? It is not a big at all, I've set "minor" flag to it. :) We discussed it with Viktor recently and both agreed that it will make our code cleaner and easier to "type". And to refactor this, I will spend no more than a few tens of minutes.
A compromise would be "import topology_m2 as topo" or something like that, which is shorter but at least still indicative or a partial word. I agree with you, I don't like and don't use such variables too. Topology is a bit another thing. It has its "main" part of the name i.e. "master", "consumer etc. So in my opinion, {{{ t.ms["master1"] }}} is still very readable. Though I do fully agree to the "'''topo'''" too, it is still better than "'''topology_m1h1c1'''".
Ultimately, is this really a big problem for us? What will this solve for us? How does it help us? It is not a big at all, I've set "minor" flag to it. :) We discussed it with Viktor recently and both agreed that it will make our code cleaner and easier to "type". And to refactor this, I will spend no more than a few tens of minutes.
Yep, I'm convinced now. Let's go with your plan. The reason I say topo not "t", is search/replace on "t" may not easily work, but at least topo is somewhat unique in a search.
attachment 0001-Ticket-49085-Make-a-short-topology-fixture-alias.patch
To ssh://git.fedorahosted.org/git/389/ds.git c969a82..9fcfb6c master -> master commit 9fcfb6c Author: Simon Pichugin spichugi@redhat.com Date: Mon Jan 16 13:47:41 2017 +0100
Metadata Update from @spichugi: - Issue assigned to spichugi - Issue set to the milestone: lib389 1.0.4
389-ds-base is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in 389-ds-base's github repository.
This issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/2144
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: Fixed)
Login to comment on this ticket.