These are chat archives for translate/dev

4th
May 2016
phlax
@phlax
May 04 2016 09:29
@/all ive addressed all feedback, refactored and rebased translate/pootle#4697 - gtm?
phlax
@phlax
May 04 2016 09:35
seems like https://docs.djangoproject.com/en/1.8/ and its killing tests 8(
Taras Semenenko
@ta2-1
May 04 2016 09:45
@phlax what did you mean re killing tests?
phlax
@phlax
May 04 2016 09:51
intersphinx failure
phlax
@phlax
May 04 2016 10:04
cool, tests are passing and at 100% - any objections to landing translate/pootle#4697 ?
phlax
@phlax
May 04 2016 10:11
@ta2-1 re index question
there is an index_together on ct,object_pk
and an index on ct
i think they are all defo necessary
do think the index on object_pk is unnecessary?
i have a feeling it is but not entirely sure
so thought safer to add
Taras Semenenko
@ta2-1
May 04 2016 10:13
the only thing when it could be useful is when DB doesn't support indexing together.
phlax
@phlax
May 04 2016 10:14
k, shall i just remove then?
Taras Semenenko
@ta2-1
May 04 2016 10:15
yes, let's remove it
phlax
@phlax
May 04 2016 10:15
cool
we can readd in future if we find it is useful for some reason
Taras Semenenko
@ta2-1
May 04 2016 10:15
sure
Julen Ruiz Aizpuru
@julen
May 04 2016 10:32
oh nice, I'm late to the party
can someone shed some light if there was any prior discussion on this config code? thanks
phlax
@phlax
May 04 2016 10:35
@julen some, but i did bring it up with you yesterday
and didnt get the impression that there was any immediate pushback
Julen Ruiz Aizpuru
@julen
May 04 2016 10:36
phlax: I haven't had any time to look at it TBH
phlax
@phlax
May 04 2016 10:36
as explained, as i have spent so many months dealing with critical install/migrate/performance/sec issues
we really need to land some features quickly
but its part of the ongoing push to make pootle extensible/pluggable
on that note
@/all i would like to land translate/pootle#4698 asap
Julen Ruiz Aizpuru
@julen
May 04 2016 10:38
generally speaking it wouldn't hurt to share plans and discussions with enough time in advance
phlax
@phlax
May 04 2016 10:38
test coverage is at 100% and im thinking its ready to land
ive added a separate PR for docs, but would encourage feedback/contributions to the docs on the wiki pages
@julen im doing my best really
also, shortly after that i would really like to land translate/pootle#4677
part of the urgency @julen is that i want to include these components in the beta release
Leandro Regueiro
@unho
May 04 2016 10:44
@/all I am landing translate/pootle#4700 after the gtms.
phlax
@phlax
May 04 2016 10:45
great
Julen Ruiz Aizpuru
@julen
May 04 2016 10:46
my 2c: maybe it's not immediately obvious to you folks, but not everyone (besides TH) is aware of such schedules and plans, which makes it increasingly difficult to understand and take part in ongoing developments
phlax
@phlax
May 04 2016 10:48
@julen apologies
its not like we are keeping back information, i really do try and share with you at earliest opportunities
but its been a changing picture, as weve found and dealt with various issues
and now after not releasing for 6 months and having a backlog of implementations that we should have addressed
we are having to do things pretty agile
phlax
@phlax
May 04 2016 10:53
@julen one thing i had been wondering if would be helpful
is if we made our ~weekly triage meeting public
times/frequency has been pretty changeable
but we could defo fix it
Julen Ruiz Aizpuru
@julen
May 04 2016 11:44
anything affecting development being open by default would definitely help
phlax
@phlax
May 04 2016 11:58
cool, we had set a date for weds, but in reality we have been holding it when we were free
we did a triage yesterday mostly because of release issues
so shall we set a time for next weds afternoon?
@/all ^^ in relation to public bug/PR triage
we tend to voice and use jitsi, but that should be fine
Taras Semenenko
@ta2-1
May 04 2016 12:01
@phlax, does public triage meeting mean that @julen will attend such meetings?
phlax
@phlax
May 04 2016 12:02
it would certainly be useful, but i guess anyone who want to take tickets really
Julen Ruiz Aizpuru
@julen
May 04 2016 12:28
is there a way to have codecov-io reports not as PR comments? not sure when they were added, and frankly I find them quite intrusive. Reporting à-la Travis would be enough IMO
phlax
@phlax
May 04 2016 12:28
ive beene wondering the same, they have been changing their hookup recently, and its a bit intrusive
that said
the new graph widget is pretty amazing
and the hookup means you can see the cov inline, i use coveralls for most other things (a lot)
@julen can we give it few days
and see if its a bit less intrusive by then
Julen Ruiz Aizpuru
@julen
May 04 2016 12:33
how would that change within a few days? :?
phlax
@phlax
May 04 2016 12:33
well its diff today than it was yesterday
and ditto the day before
last night it was making all the PRs "fail"
but it is really useful
so if poss i want to keep hookup, not sure if you can configure it though
Julen Ruiz Aizpuru
@julen
May 04 2016 12:34
Travis-style reporting gives a good balance IMO, and we're not left behind with whatever the service decides is good the day after
phlax
@phlax
May 04 2016 12:35
yep, i dont think they have worked out the communicaiton of their features very well yet
but the features are pretty killer
as said i use coveralls 90% of the time
but codecov has a few things that coveralls doesnt
apart from the graphs, and the inline coverage highlighting
Julen Ruiz Aizpuru
@julen
May 04 2016 12:36
so I guess you already have a permanent tab open with it
phlax
@phlax
May 04 2016 12:36
yep 8/
the other killer feature on codecov
is that coveralls doesnt show properly for newly added files
that said, i love the simplicity of coveralls, so use that mostly
but
if they dont tone down the hookup, then im also thinking to remove it
Julen Ruiz Aizpuru
@julen
May 04 2016 12:41
even if it becomes less intrusive, I'd kindly ask to disable the PR comments. When trying to go through and manage GH notifications, it only adds salt to the injury, as it's excessive noise which can be avoided without having to disable the service altogether
phlax
@phlax
May 04 2016 12:45
hmm, just writing some command docs and wonder if you do this for .. django-admin-option:: --foo how should you specify the short-form -f or similar, @unho do you know?
Julen Ruiz Aizpuru
@julen
May 04 2016 12:51
phlax: haven't tried, but I believe you can use a comma-separated list as in .. django-admin-option:: --foo, -f
phlax
@phlax
May 04 2016 12:51
nm, seems like added in dj1.9 https://code.djangoproject.com/ticket/24134
Julen Ruiz Aizpuru
@julen
May 04 2016 12:52
phlax: I might be wrong, but I don't think that's django-admin-option
check _ext/pootle_docs.py, the custom directive is based on cmdoption http://certik.github.io/sphinx/markup/desc.html#dir-cmdoption
phlax
@phlax
May 04 2016 12:53
thanks, will do
from what i can see that is just for django-admin
but dont know enough sphinx to be sure
apologies
i just saw it
if all good im hoping to land the PR (translate/pootle#4698) shortly
Leandro Regueiro
@unho
May 04 2016 13:15
@phlax Reviewed translate/pootle#4698
phlax
@phlax
May 04 2016 13:15
thanks @unho ill update with your feedback
@unho did you get a chance to look at the cli docs?
Leandro Regueiro
@unho
May 04 2016 13:17
Not yet. Doing now.
phlax
@phlax
May 04 2016 13:18
ta, feel free to update directly if you spot anything
and it needs to run through sphinx to see that it works
Julen Ruiz Aizpuru
@julen
May 04 2016 13:18
unho: I see you commented that CLI texts need to be wrapped in gettext calls. FWIW that's not the case and never been the case
phlax
@phlax
May 04 2016 13:19
@julen they dont need to be?
to be fair @unho said he wasnt sure
Leandro Regueiro
@unho
May 04 2016 13:19
Yes, wasn't sure because those are administrator-facing texts.
phlax
@phlax
May 04 2016 13:19
but surely this could be localised - no?
if you were non-english speaking admin
Leandro Regueiro
@unho
May 04 2016 13:20
It can. But the questions is if it should.
I hardly doubt it.
phlax
@phlax
May 04 2016 13:20
@julen why not?
Julen Ruiz Aizpuru
@julen
May 04 2016 13:21
Regardless of the whether it should/not question, we don't. So keeping that consistent is a good enough reason not to do it right now
Leandro Regueiro
@unho
May 04 2016 13:22
@phlax I am quickly checking and no command seems to be doing it.
phlax
@phlax
May 04 2016 13:22
hmm
im thinking that is something that we should discuss
Julen Ruiz Aizpuru
@julen
May 04 2016 13:22
not definitely in this context
phlax
@phlax
May 04 2016 13:22
i dont have strong feelings, but it just seems strange that we dont
Leandro Regueiro
@unho
May 04 2016 13:24
To me it seems strange to do. Not only because we will display localized messages to admins who would surely understand english, but because we might get bizarre bug reports. Also this will increase the strings to translate.
phlax
@phlax
May 04 2016 13:25
re the 2nd point - that is a good point
re the 1st and i dont see any reason at all to presume that sysadmin speaks english other than we have not properly localised our software
i have some uncertainty here because it makes a tool imprecise and/or unpredictable eg in scripting
Leandro Regueiro
@unho
May 04 2016 13:27
If we are talking only about Linux sysadmins I find it hard to believe. Also if we localize these messages there is also a point for localizing documentation. And for now we have more pressing issues, IMHO.
phlax
@phlax
May 04 2016 13:28
but im really not sure what the "right" thing is
so i opened a ticket to let more knowledgeable speak on it
ill update the PR now to remove the gettext wrappers
Leandro Regueiro
@unho
May 04 2016 13:30
Thanks.
phlax
@phlax
May 04 2016 13:35
re keys, it is kinda correct, it can be specified multiple times
re other bug
can you edit the wiki
ta
ill add something re keys
Leandro Regueiro
@unho
May 04 2016 13:37
editing
phlax
@phlax
May 04 2016 13:38
@unho you can treat the wiki as a repo incidentally
if you do it helps to work in branch
even without PRs etc
phlax
@phlax
May 04 2016 13:47
@unho lmk when you have finished with docs
and ill copy into PR and test with sphinx
Leandro Regueiro
@unho
May 04 2016 13:47
@phlax I am finished.
phlax
@phlax
May 04 2016 13:47
thanks
Julen Ruiz Aizpuru
@julen
May 04 2016 14:00
I see the same gettext thing applies to config model's verbose names. Unless these are used in the UI (which I really doubt it's the case), it only contributes to add irrelevant localizable strings
phlax
@phlax
May 04 2016 14:01
@julen could you make that point on the ticket i opened
Julen Ruiz Aizpuru
@julen
May 04 2016 14:02
this is orthogonal to CLI output
phlax
@phlax
May 04 2016 14:02
hmm
well its kinda related i reckon, but yep CLI output is a special case
my feeling is that we should localise everything that it specifically doenst make sense too
but i would defo like to hear @dwaynebailey 's thoughts on this
because i figure he will have thought about it
Julen Ruiz Aizpuru
@julen
May 04 2016 14:04
verbose names that aren't displayed to users are irrelevant for localizers, and extra burden which should be avoided
phlax
@phlax
May 04 2016 14:04
it kinda deps on the app really
there is quite a few sysadmin reasons that it would be displayed
but
perhaps it should have only one name
Julen Ruiz Aizpuru
@julen
May 04 2016 14:05
nope, if no one ever sees a message, there's no point in forcing people to do extra work
as a localizer I can tell you how discouraging it is having to localize text which won't be used at all (extra points if it contains technical jargon)
Leandro Regueiro
@unho
May 04 2016 14:07
+1
phlax
@phlax
May 04 2016 14:07
hmm, as said i dont have strong feelings
but i do think its something that we need a clear policy on, and perhaps even some explanation that would assist other people working on software localisation
Julen Ruiz Aizpuru
@julen
May 04 2016 14:15
it seems like you are mixing concerns — at least I was talking about model verbose names
phlax
@phlax
May 04 2016 14:16
thanks for feedback
Julen Ruiz Aizpuru
@julen
May 04 2016 14:47
having checked the config code, it reminds me of the ability of Sentry plugins to get/set arbitrary options
phlax
@phlax
May 04 2016 14:48
is that a good thing?
Julen Ruiz Aizpuru
@julen
May 04 2016 14:48
it's just a fact
I find Sentry's API more natural though
config.get().set_config() doesn't feel natural to me
phlax
@phlax
May 04 2016 14:49
as i mentioned to @ta2-1 i really like zope.IAnnotations
that has the best/most extensible api i have used for "writing on" other objects
but we sadly are not in a world of Interfaces
phlax
@phlax
May 04 2016 14:51
well the reason for using the explict *_config
is that its a queryset
so i wanted to ensure these methods were part of a specific/separate api
Julen Ruiz Aizpuru
@julen
May 04 2016 14:54
I haven't found this in tests, and I wonder what's the behavior if two different plugins use the same configuration key for the same object/namespace
phlax
@phlax
May 04 2016 14:54
well if you look in the store_serializers PR
that is why it uses explicit pootle.core.serializers
as the key
Julen Ruiz Aizpuru
@julen
May 04 2016 14:55
you are probably familiar with it but I'm not. Do you have a pointer?
phlax
@phlax
May 04 2016 14:55
yep, sorry one mo
but that key is used throughout the PR
Julen Ruiz Aizpuru
@julen
May 04 2016 14:59
Sentry plugins handle this semi-transparently by prepending a prefix to keys
phlax
@phlax
May 04 2016 14:59
that is what i have done
ie pootle.core
that is the authoritative namespace
but tbh @julen its designed specifically to make it usable in the way you want
in terms of pootle app settings, my feeling is to use the namespace of the thing its point to
in this case its pootle.core.delegate.serializers
but i think the delegate is superfluous
Julen Ruiz Aizpuru
@julen
May 04 2016 15:02
that's good, however in many cases you won't need all the bells and whistles and having a set of defaults that just work will make life easier for potential plugin authors
phlax
@phlax
May 04 2016 15:02
well
im pretty happy with the implementation in that it allows devs to use as they feel
but to be useful to other devs then it needs to point to the namespace which cares about this setting
and in some cases that is not immediately clear or is ambiguous
so it needs a bit of common sense really
rather than a programmatic solution i feel
Julen Ruiz Aizpuru
@julen
May 04 2016 15:05
another thing I don't get is why is this in a separate app, when my understanding is this is a fundamental piece built for core plugins
phlax
@phlax
May 04 2016 15:06
that is why the spec is in pootle.core.delegate
having in a separate app
means that end users can customize the functionality
eg use a diff Config model, or use a diff Config model just for some models (eg with an int object_pk)
Julen Ruiz Aizpuru
@julen
May 04 2016 15:07
FWIW it doesn't need to be in a separate app for enabling that (and I don't immediately see the use case)
phlax
@phlax
May 04 2016 15:07
it does in terms of placing something in between in INSTALLED_APPS
Julen Ruiz Aizpuru
@julen
May 04 2016 15:08
mind elaborating?
phlax
@phlax
May 04 2016 15:09
apologies no time just now
but im happy to explain later or tomorrow
Julen Ruiz Aizpuru
@julen
May 04 2016 15:11
ok no worries. I'll be off for the long weekend, you can elaborate next week if you want
phlax
@phlax
May 04 2016 15:11
just have to go afk v soon and trying to finish something
phlax
@phlax
May 04 2016 15:31
@ta2-1 ive made the column widths dynamic on the config cli tool
Taras Semenenko
@ta2-1
May 04 2016 15:31
thanks
phlax
@phlax
May 04 2016 15:31
@/all feedback addressed on translate/pootle#4698 any blockers to landing?
@julen master is broken
Taras Semenenko
@ta2-1
May 04 2016 15:34
@phlax as I understand we are to remove all gettext from cli output, yes?
phlax
@phlax
May 04 2016 15:35
that seems the consensus
Taras Semenenko
@ta2-1
May 04 2016 15:35
you have _() for help attribute of config command
Leandro Regueiro
@unho
May 04 2016 15:35
I was about to ask the same. I still see gettext being used.
phlax
@phlax
May 04 2016 15:36
sorry can you print line
ill change now, i was just about to push
nm
Leandro Regueiro
@unho
May 04 2016 15:36
@phlax Just open the file and search for _(
phlax
@phlax
May 04 2016 15:36
it has help messages
they should be localised
and error message
Leandro Regueiro
@unho
May 04 2016 15:37
If you don't localize the error messages, you shouldn't be localizing any part of the command at all.
phlax
@phlax
May 04 2016 15:37
that also shoud be localised
eh
surely we do localise error messages - no?
Leandro Regueiro
@unho
May 04 2016 15:37
No command in Pootle is using Gettext at all.
phlax
@phlax
May 04 2016 15:37
i can remove but im surprised by that
im defo not up for removing from help messages
Leandro Regueiro
@unho
May 04 2016 15:38
I thought that was what we agreed on.
phlax
@phlax
May 04 2016 15:38
that is wrong
and we agreed re output
not re error messages
Leandro Regueiro
@unho
May 04 2016 15:38
As Julen said please align this with the rest of the commands.
Help messages are output.
phlax
@phlax
May 04 2016 15:39
that is really wrong
k im going to change and land
but its completely english-centric
help should help in any language
Leandro Regueiro
@unho
May 04 2016 15:39
For me is exactly the same use case. This is only used by admins, so no point on making this translatable.
phlax
@phlax
May 04 2016 15:39
no point
what is the point of localisation at all @unho?
Taras Semenenko
@ta2-1
May 04 2016 15:43
let's talk about it in translate/pootle#4703
phlax
@phlax
May 04 2016 15:43
fine
localisation removed (er?) gtm @/all ?
phlax
@phlax
May 04 2016 18:06
@/all the serialization PR is updated and hopefully ready to land - translate/pootle#4677
reviews/gtm welcome 8/
i have separated out the dev docs re serializers for now, and will post on the wiki so we can edit collaboratively
phlax
@phlax
May 04 2016 19:07
the serializer docs were already there actually https://github.com/translate/pootle/wiki/DocsDevSerializers