1 2014-05-26T00:07:21 *** derdon
2 2014-05-26T02:12:53 *** sl33k_
3 2014-05-26T02:17:17 *** sl33k_
4 2014-05-26T02:59:58 *** aboutGod
5 2014-05-26T03:05:04 *** aboutGod
6 2014-05-26T05:08:05 *** sl33k_
7 2014-05-26T08:57:19 *** greg_f
8 2014-05-26T09:05:58 *** randomax
9 2014-05-26T09:34:22 <randomax> ThomasWaldmann, how do i create a wikifarm ?
10 2014-05-26T09:41:45 <randomax> also removed multiple occurrences of the word "settings" - https://codereview.appspot.com/95600044
11 2014-05-26T10:46:59 *** randomax
12 2014-05-26T11:12:25 *** skathpalia
13 2014-05-26T11:20:01 <skathpalia> moin
14 2014-05-26T11:21:44 <dimazest> hello
15 2014-05-26T11:23:15 <skathpalia> dimazest, ThomasWaldmann I searched for the places where this method https://bitbucket.org/thomaswaldmann/moin-2.0/src/4a997d9f5e26515b90c744582be8aa7393bb57d1/MoinMoin/forms.py?at=default#cl-352 is used and according to me it is used only for ticket items for producing select lists such as Depends On, Superseded By and Assigned To
16 2014-05-26T11:24:11 <skathpalia> So I think my cr https://codereview.appspot.com/100720043/ would work fine as this method is not used at any other place
17 2014-05-26T11:27:00 <dimazest> ok
18 2014-05-26T11:27:19 <dimazest> but is the implemented behavior desirable
19 2014-05-26T11:27:52 <dimazest> do we want to switch to item_id if some revision doesn't have a name
20 2014-05-26T11:28:21 <skathpalia> Here we are specifically talking of ticket items
21 2014-05-26T11:28:48 <dimazest> yes
22 2014-05-26T11:28:59 <skathpalia> So we have to stick with itemid only because only that way one can access the ticket
23 2014-05-26T11:29:33 <dimazest> but if it has a name, we will use it?
24 2014-05-26T11:29:47 <dimazest> do we need that try .. except block
25 2014-05-26T11:30:05 <dimazest> or we can always show shortened item_ids
26 2014-05-26T11:30:19 <skathpalia> Also if some tickets have name then it be difficult for the user to distinguish between the name and ITEMID (especially if name is also of length 32) in the select list
27 2014-05-26T11:31:22 <skathpalia> Yeah we will try to shorten the itemids
28 2014-05-26T11:33:50 <skathpalia> We will try to do this the same way it is done in History view
29 2014-05-26T11:35:16 <skathpalia> dimazest, Can I commit it no to my repo?
30 2014-05-26T11:36:00 <dimazest> did you answer to ThomasWaldmann comment?
31 2014-05-26T11:36:09 <dimazest> there is also a pep8 issue
32 2014-05-26T11:36:20 <skathpalia> Yeah I corrected that
33 2014-05-26T11:36:58 <dimazest> maybe, having instead of the current code
34 2014-05-26T11:37:10 <dimazest> choices = [(rev.meta[ITEMID], (rev.meta[ITEMID]) for rev in revs]
35 2014-05-26T11:37:16 <dimazest> would make more sense
36 2014-05-26T11:37:27 <dimazest> then the template could be simplified
37 2014-05-26T11:37:58 <dimazest> * there is an extra ( in the code
38 2014-05-26T11:40:27 <skathpalia> But how would selecting same thing 2 times would help?
39 2014-05-26T11:42:52 <skathpalia> dimazest, also I don't see any extra ( in the code. Can you please point to that place ?
40 2014-05-26T11:43:03 <dimazest> in the code i posted
41 2014-05-26T11:43:36 <skathpalia> Oh I thought it was in my code
42 2014-05-26T11:43:48 <dimazest> if we want to show item_id, why do we need to have code, that tries to retrieve tickets names?
43 2014-05-26T11:44:15 <skathpalia> Not in every field
44 2014-05-26T11:44:34 <skathpalia> We want to show names in Assigned to field
45 2014-05-26T11:44:51 <skathpalia> for superseded by and depends on we need to show ITEMIDS
46 2014-05-26T11:44:54 *** randomax
47 2014-05-26T11:45:29 <dimazest> so, than the code makes an assumption that tickets won't have name
48 2014-05-26T11:46:19 <dimazest> wouldn't it be better to have a parameter, that explicitly says, that name or item_id should be used
49 2014-05-26T11:46:23 <dimazest> without any magic
50 2014-05-26T11:47:55 <skathpalia> for that we need to get the field name for which it is used
51 2014-05-26T11:48:27 <skathpalia> I also tried for that but I couldn't find hoe to get the field name
52 2014-05-26T11:48:49 <skathpalia> dimazest, I think this method is also fine :)
53 2014-05-26T11:49:11 <dimazest> well, if it was fine, there would not be a comment about it's semantics
54 2014-05-26T11:50:41 <skathpalia> The semantics in that comment was pointing to selecting either all name or all ITEMIDs
55 2014-05-26T11:51:10 <dimazest> yes, and you got the same comment twice
56 2014-05-26T11:51:40 <dimazest> it's rather strange code, and it's not really clear what it does
57 2014-05-26T11:52:26 <dimazest> so
58 2014-05-26T11:52:36 <dimazest> the template which uses that function
59 2014-05-26T11:52:37 <dimazest> MoinMoin/templates/forms.html
60 2014-05-26T11:52:42 <skathpalia> Sorry for that But this code is just making options for the select list for ticket items
61 2014-05-26T11:53:09 <dimazest> wait is it only about ticket items?
62 2014-05-26T11:53:13 <skathpalia> and sometimes it sees for names and sometimes for ITEMIDs
63 2014-05-26T11:53:19 <dimazest> then why do we care about item names?
64 2014-05-26T11:53:27 <skathpalia> Yeah
65 2014-05-26T11:54:06 <dimazest> why do we need to try to retrieve item NAME?
66 2014-05-26T11:54:09 <skathpalia> Yeah exactly thats why we are just showing ITEMIDs
67 2014-05-26T11:54:32 <dimazest> but in the try ... except we first try to get names
68 2014-05-26T11:54:37 <skathpalia> We are trying to retrieve names for Assigned To select list
69 2014-05-26T11:54:43 <dimazest> and if fail, only the fall back to item id's
70 2014-05-26T11:55:34 <skathpalia> Assigned select list should have names of user
71 2014-05-26T11:55:35 <dimazest> so, maybe Assigned to select list should use a different macro
72 2014-05-26T11:56:03 <dimazest> or a different method of retrieving names/ids
73 2014-05-26T11:57:43 <skathpalia> Then we have to make a new template also because for now we are using standard template forms.html
74 2014-05-26T11:58:26 <dimazest> we can inherit from the standart one
75 2014-05-26T11:58:34 <dimazest> *standard
76 2014-05-26T11:58:49 <dimazest> and redefine the only bits that have to be changed
77 2014-05-26T11:59:24 <skathpalia> Yeah may be this can be done
78 2014-05-26T12:00:23 <dimazest> that, imho, would be much more clear
79 2014-05-26T12:00:48 <skathpalia> But for now we can go with this as we are still left to make notifications part also work properly which is very important
80 2014-05-26T12:01:13 <skathpalia> for ticket items and also other items to work properly
81 2014-05-26T12:03:16 <skathpalia> What is your opinion about this?
82 2014-05-26T12:03:19 <dimazest> but then this is nice solution, but rather a hack
83 2014-05-26T12:04:12 <dimazest> then i would expect a comment that describes the problem with the proposed solution and gives a hint how to solve it properly
84 2014-05-26T12:04:25 <skathpalia> Yeah making a separate template would be better
85 2014-05-26T12:05:02 <skathpalia> You mean comment in the codereview?
86 2014-05-26T12:05:14 <dimazest> no, in the code
87 2014-05-26T12:05:35 <dimazest> as was done with that assert regarding name
88 2014-05-26T12:06:05 <skathpalia> Okay will add a TODO in the code regarding your proposed solution
89 2014-05-26T12:06:24 <skathpalia> Then I will start working on the notifications part :)
90 2014-05-26T12:06:30 <dimazest> ok
91 2014-05-26T12:06:42 <dimazest> ThomasWaldmann: what do you think?
92 2014-05-26T12:47:05 *** skathpalia
93 2014-05-26T12:59:33 *** skathpalia
94 2014-05-26T13:33:04 *** randomax
95 2014-05-26T13:48:15 *** RogerHaase
96 2014-05-26T14:05:42 *** randomax
97 2014-05-26T14:10:20 <RogerHaase> randomax: creating a wiki farm on moin2 is not well-documented nor likely well-tested. So rather than expand your scope to include testing wiki farm, just find an appropriate spot to add a comment in the code something like: #TODO revisit contents of quicklink tooltips when wiki farms, namespaces, and/or nameless tickets become available
98 2014-05-26T14:11:59 *** magu_cic
99 2014-05-26T14:19:27 *** randomax
100 2014-05-26T14:56:26 *** sl33k_
101 2014-05-26T15:24:48 *** randomax
102 2014-05-26T15:30:46 *** magu_cic_
103 2014-05-26T15:34:29 *** magu_cic
104 2014-05-26T15:54:44 *** skathpalia
105 2014-05-26T15:54:58 *** skathpalia
106 2014-05-26T16:25:43 <ThomasWaldmann> moin
107 2014-05-26T16:30:36 <skathpalia> ThomasWaldmann, I searched for the places where this method https://bitbucket.org/thomaswaldmann/moin-2.0/src/4a997d9f5e26515b90c744582be8aa7393bb57d1/MoinMoin/forms.py?at=default#cl-352 is used and according to me it is used only for ticket items for producing select lists such as Depends On, Superseded By and Assigned To
108 2014-05-26T16:31:34 <skathpalia> So I think my cr https://codereview.appspot.com/100720043/ would work fine
109 2014-05-26T16:35:38 <skathpalia> I have also corrected that pep8 error in this cr
110 2014-05-26T16:38:21 <RogerHaase> randomax: what are you working on now?
111 2014-05-26T16:45:04 <ThomasWaldmann> dimazest: skathpalia: i'ld prefer a clean solution. hacks only make more troubles in future.
112 2014-05-26T16:45:31 <ThomasWaldmann> for tickets, dealing with names is obviously pointless if we know there are none.
113 2014-05-26T16:46:01 <dimazest> ThomasWaldmann: what do you think is a good solution?
114 2014-05-26T16:46:19 <ThomasWaldmann> but just showing itemids is often not giving enough information to the users
115 2014-05-26T16:46:47 <skathpalia> One thing can be done that we can show one-line summary also along with the itemid
116 2014-05-26T16:46:49 <ThomasWaldmann> so there somehow should be a means to show more information that is fetched from the metadata
117 2014-05-26T16:47:29 <dimazest> a custom template seems like a good idea then
118 2014-05-26T16:47:33 <ThomasWaldmann> if we have a name as unique key, it should be able to use that and not itemid
119 2014-05-26T16:47:49 <ThomasWaldmann> caller decides.
120 2014-05-26T16:48:24 <skathpalia> But the tickets don't have name so how can we take name as key?
121 2014-05-26T16:48:48 <ThomasWaldmann> (do not just think of tickets. a wiki item could use same code for "i am referencing to ..." or "i am referenced by ...")
122 2014-05-26T16:49:13 <ThomasWaldmann> the caller knows what's possible in his context
123 2014-05-26T16:49:40 <skathpalia> But I have checked that this code is used only for the ticket items
124 2014-05-26T16:49:42 <ThomasWaldmann> so if the caller is ticket code, it will say "use itemid". if it is wiki code, it will say "use name"
125 2014-05-26T16:50:11 <ThomasWaldmann> that's the current state, but not engraved into stone
126 2014-05-26T16:50:20 <dimazest> can we pass this information to _get_choice_specs()
127 2014-05-26T16:50:29 <dimazest> from a template?
128 2014-05-26T16:50:32 <skathpalia> Also if in future some other wiki also uses that code then also it will first look for names only and then ITEMID
129 2014-05-26T16:52:30 <ThomasWaldmann> well, if caller says he wants names and you give back itemids, that should not just magically return strings with different meanings
130 2014-05-26T16:53:21 <skathpalia> the output would be names only if the item is a named item
131 2014-05-26T16:53:54 <skathpalia> only in the case of items not having name ITEMIDs will be returned
132 2014-05-26T16:55:02 * ThomasWaldmann looks at the badly commented original code
133 2014-05-26T16:55:03 <skathpalia> Also while calling for names the caller should make sure that name is there in the meta of that particular item
134 2014-05-26T17:01:45 <RogerHaase> In addition to quick Links, will a user be able to subscribe to a ticket and what will be shown as evidence of the subscription in user settings form? An item ID is not very user friendly.
135 2014-05-26T17:03:47 <skathpalia> Presently user is not able to subscribe to ticket items as the Notification module and the subscription module are dependent on item_name which is not there in case of tickets
136 2014-05-26T17:04:53 <skathpalia> RogerHaase, If this feature is added then ITEMID will be shown in the settings form as it is only used for referencing ticket
137 2014-05-26T17:05:48 <skathpalia> Yeah it is not user friendly but what else can be added there to make it better
138 2014-05-26T17:07:40 <ThomasWaldmann> i think that the identifier (tuple[0]) of course always should be itemid
139 2014-05-26T17:07:56 <ThomasWaldmann> tuple[1] is used as visible text for the user, right?
140 2014-05-26T17:08:38 <ThomasWaldmann> so it should come from metadata and the key should be given when creating the class instance
141 2014-05-26T17:09:13 <ThomasWaldmann> there's this .using(...) and .with_properties(...) stuff
142 2014-05-26T17:09:30 <skathpalia> Yeah tuple[1] is visible text and if is not found then it visible text becomes tuple[0]
143 2014-05-26T17:10:32 <dimazest> but it's done with crazy "or" magic
144 2014-05-26T17:10:57 <dimazest> it would be nice to get rid of it, and move the logic out of the template to the form
145 2014-05-26T17:19:22 <skathpalia> ThomasWaldmann, what type of key are you talking about?
146 2014-05-26T17:21:15 <ThomasWaldmann> the key to access the metadata item we want to show as label
147 2014-05-26T17:21:53 <ThomasWaldmann> btw, Reference is used at 2 places: for users and for tickets
148 2014-05-26T17:22:16 <ThomasWaldmann> so we already have 1 case where we need key=ITEMID and one where we need key=NAME
149 2014-05-26T17:23:55 <skathpalia> in case of users key would be name and in case of tickets the key would be ITEMID
150 2014-05-26T17:24:17 <skathpalia> This would be taken care by that try except statements
151 2014-05-26T17:24:45 <ThomasWaldmann> no, the choice is made by caller
152 2014-05-26T17:25:08 <ThomasWaldmann> not by what's found in metadata and try/except
153 2014-05-26T17:26:11 <skathpalia> Then we have to pass a parameter to _get_choice_specs method specifying which key to use
154 2014-05-26T17:26:13 <skathpalia> right?
155 2014-05-26T17:27:18 <ThomasWaldmann> maybe cls.properties could be used. not sure about best way.
156 2014-05-26T17:28:30 <skathpalia> Can you give me an idea how we can use cls.properties to achieve that?
157 2014-05-26T17:28:41 <ThomasWaldmann> use grep ;)
158 2014-05-26T17:29:40 <ThomasWaldmann> it's already used in _get_choice_specs
159 2014-05-26T17:30:41 <skathpalia> But that is just used for adding adding empty option in the select list
160 2014-05-26T17:31:49 <ThomasWaldmann> yeah, but that doesn't hold you back adding another property for what you need
161 2014-05-26T17:32:15 <ThomasWaldmann> e.g. label_meta_key
162 2014-05-26T17:32:26 <skathpalia> Yeah i think I got it
163 2014-05-26T17:34:18 <skathpalia> I have one more doubt that how can I get that the field for which this code is executed
164 2014-05-26T17:34:37 <ThomasWaldmann> and if that list comprehension is not flexible enough for what we need to do, I suggest you use a loop
165 2014-05-26T17:34:53 <skathpalia> I mean whether is it for Assigned To or Superseded By or Depends On
166 2014-05-26T17:36:12 <skathpalia> Because according to that I have to pass this label_meta_key
167 2014-05-26T17:36:14 <ThomasWaldmann> the caller knows that if we deal with users or wiki items, we want label_meta_key=NAME, if we deal with ticket items, we want =ITEMID
168 2014-05-26T17:37:52 <ThomasWaldmann> OptionalUserReference = Reference.to(USER_QUERY).using(optional=True).with_properties(empty_label='(Nobody)')
169 2014-05-26T17:37:56 <skathpalia> So I need to check in the revs and see of it is a ticket item or other
170 2014-05-26T17:37:59 <skathpalia> right?
171 2014-05-26T17:38:05 <ThomasWaldmann> ^^ that's the caller
172 2014-05-26T17:38:51 <ThomasWaldmann> see ticket.py:~39
173 2014-05-26T17:39:03 <ThomasWaldmann> no
174 2014-05-26T17:40:35 <skathpalia> Okay so I will add one label_meta_key and accordingly check if it is a user or a ticket reference
175 2014-05-26T17:40:42 <skathpalia> Will try it
176 2014-05-26T17:40:49 <skathpalia> Thanks ThomasWaldmann :)
177 2014-05-26T17:41:56 <ThomasWaldmann> the code doesn't need to "check", the coder will decide what's needer
178 2014-05-26T17:42:04 <ThomasWaldmann> needed*
179 2014-05-26T17:46:20 <skathpalia> What I am thinking is that I will add OptionalTicketReference = Reference.to(TICKET_QUERY).using(optional=True).with_properties(label_meta_key=TICKET) and then in forms.py this property is checked
180 2014-05-26T17:46:54 <skathpalia> and accordingly choice_specs is made
181 2014-05-26T17:47:15 <skathpalia> ThomasWaldmann, Is this method right?
182 2014-05-26T17:48:17 <skathpalia> Also will add label_meta_key in the corresponding UserReference part also with label_meta_key=USER
183 2014-05-26T17:48:50 <ThomasWaldmann> TICKET is a key in metadata?
184 2014-05-26T17:49:50 <ThomasWaldmann> USER is a key in metadata?
185 2014-05-26T17:50:37 <skathpalia> Okay we can have boolean value for this label_meta_key
186 2014-05-26T17:50:45 <ThomasWaldmann> no
187 2014-05-26T17:51:16 <ThomasWaldmann> you give a key there which you can use to get meta[key]
188 2014-05-26T17:51:27 *** greg_f
189 2014-05-26T17:53:01 <skathpalia> I am not able to understand how exactly it will solve our problem which is that we have to check whether it is a ticket reference or other reference
190 2014-05-26T17:53:08 <skathpalia> Can you please explain it?
191 2014-05-26T17:53:19 <ThomasWaldmann> you won't check
192 2014-05-26T17:53:53 <ThomasWaldmann> OptionalTicketReference =
193 2014-05-26T17:53:58 <skathpalia> Then how can we differentiate between the two types of references?
194 2014-05-26T17:54:12 <ThomasWaldmann> Reference.to(TICKET_QUERY).using(optional=True).with_properties(label_meta_key=ITEMID)
195 2014-05-26T17:56:20 <skathpalia> Okay then we can simply do this in forms.py choices = [(rev.meta[ITEMID], rev.meta[cls.[label_meta_key]]) for rev in revs]
196 2014-05-26T17:56:26 <skathpalia> I mean something like that
197 2014-05-26T17:56:28 <skathpalia> right?
198 2014-05-26T17:56:51 <dimazest> yeap
199 2014-05-26T17:56:56 <ThomasWaldmann> if you fix the syntax, yes
200 2014-05-26T17:57:12 <skathpalia> :D
201 2014-05-26T17:57:59 <skathpalia> Sorry I was thinking something else regarding that key part so it took time for understanding it
202 2014-05-26T17:58:38 <skathpalia> Will make changes and would show you the cr by tomorrow
203 2014-05-26T17:59:06 <skathpalia> ThomasWaldmann, dimazest Also today I worked on some UI part of ticket
204 2014-05-26T17:59:33 <skathpalia> You can see the screenshot here http://picpaste.com/pics/Screenshot_from_2014-05-26_22_38_22-i9PecjPs.1401124708.png
205 2014-05-26T18:00:18 <skathpalia> In this I have defined the css for the radio buttons and input of text type
206 2014-05-26T18:01:50 <ThomasWaldmann> hmm, i don't see the difference. didn't it look like that before, too?
207 2014-05-26T18:04:03 <skathpalia> No earlier it looked like this http://picpaste.com/pics/Screenshot_from_2014-05-26_23_32_55-lVtxasEI.1401127417.png
208 2014-05-26T18:04:55 <skathpalia> ThomasWaldmann, earlier there was no defined css for radio buttons
209 2014-05-26T18:05:35 <ThomasWaldmann> new one looks a bit more dense/crowded
210 2014-05-26T18:06:23 <skathpalia> Yeah I will increase the spacing
211 2014-05-26T18:06:36 <skathpalia> Then it would look fine
212 2014-05-26T18:07:14 <skathpalia> This is happenning as the text size of labels are bigger
213 2014-05-26T18:08:04 <skathpalia> Okay will improve it :)
214 2014-05-26T18:10:19 *** skathpalia
215 2014-05-26T18:13:03 *** magu_cic
216 2014-05-26T18:15:53 *** magu_cic_
217 2014-05-26T18:30:14 <RogerHaase> randomax: or ThomasWaldmann; please close #409, fixed comment added to issue
218 2014-05-26T18:57:49 <randomax> RogerHaase, i was waiting for a review on https://codereview.appspot.com/91680043/
219 2014-05-26T18:58:20 <ThomasWaldmann> RogerHaase: done
220 2014-05-26T18:58:29 <randomax> its the patch for quicklinks and its tooltips
221 2014-05-26T18:59:22 <randomax> Then i'll start work on search UI
222 2014-05-26T19:00:08 *** randomax
223 2014-05-26T19:07:15 * ThomasWaldmann wondering about the 1:
224 2014-05-26T19:10:26 <ThomasWaldmann> RogerHaase: https://bitbucket.org/thomaswaldmann/moin-2.0/pull-request/205/fixes-issue-73-in-sharkys-repo/diff that's ok like that?
225 2014-05-26T19:12:07 <RogerHaase> yes, sidebar looks much better
226 2014-05-26T19:13:48 <ThomasWaldmann> ok
227 2014-05-26T19:13:57 * ThomasWaldmann does some merges...
228 2014-05-26T19:14:19 *** magu_cic_
229 2014-05-26T19:17:41 *** magu_cic
230 2014-05-26T19:17:46 *** derdon
231 2014-05-26T19:23:23 <ThomasWaldmann> fsck, merge conflicts in generated css :(
232 2014-05-26T19:25:08 <RogerHaase> those are fun, but if only css, ignore and rerun m css to fix
233 2014-05-26T19:27:21 <ThomasWaldmann> fixed manually
234 2014-05-26T19:27:25 <ThomasWaldmann> and pushed
235 2014-05-26T19:30:08 <ThomasWaldmann> adding remote bookmark catch-access-denied
236 2014-05-26T19:30:12 <ThomasWaldmann> huh?
237 2014-05-26T19:31:30 <RogerHaase> ThomasWaldmann: randomax's removing the empty line from common.css is my fault. Stylus is adding an extra blank line to the modernized common.css every time I compile css on windows. I forgot to run m coding-std after running m css on a 2-week old change. The extra blank line does not happen on foobar common.css nor on linux afaik.
238 2014-05-26T19:31:49 <ThomasWaldmann> ah
239 2014-05-26T19:49:18 *** magu_cic
240 2014-05-26T19:50:59 *** magu_cic_
241 2014-05-26T21:58:37 *** magu_cic
242 2014-05-26T21:59:46 *** RogerHaase
243 2014-05-26T22:00:18 *** magu_cic
244 2014-05-26T23:53:11 *** magu_cic_
245 2014-05-26T23:56:07 *** magu_cic
246
