do a favor for users who tend to use Update ID
AbandonedPublic

Authored by lnie on Jul 19 2016, 9:21 AM.

Details

Reviewers
lbrabec
kparal
jskladan
Group Reviewers
task-rpmlint
Summary

validate bodhi_id for task-rpmlint

Test Plan

run the rpmlint task with bodhi_id

Diff Detail

Repository
rRPMLINT task-rpmlint
Branch
angel
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 732
Build 732: arc lint + arc unit
lnie retitled this revision from to do a favor for users who tend to use Update ID.Jul 19 2016, 9:21 AM
lnie updated this object.
lnie edited the test plan for this revision. (Show Details)
lnie added reviewers: task-rpmlint, jskladan, kparal, lbrabec.
jskladan requested changes to this revision.Jul 19 2016, 10:44 AM

I am sorry, but this is absurd - the code, to my knowledge, does not and can not work, at least not without related libtaskotron patches that actually allow for it to work. Have you even tried running it? If so, could you tell us exactly how do you invoke it, and what libtaskotron version you use? I don't want to sound mean, but this just does not make sense to me.

First of all, you are using an undefined variable in the formula - the koji_bodhi variable (that you assume is getting populated somehow) can not be passed to the formula in any way right now, at least not from the command line. On top of that, you are trying to use koji/bodhi download quite wildly. Looking past the obvious error of using a non-existent koji_bodhi parameter, this can never work as either the koji or bodhi download action will inevitably fail, thus failing the whole rpmlint execution.

Differentiating between koji build and bodhi update in the reporting code merely based on a presence of a . in the string is _probably_ OK as we are using update-id's, but I'm not sure that it's foolproof - @kparal, is it?

This revision now requires changes to proceed.Jul 19 2016, 10:44 AM

Lili, you need to start by explaining why you're posting patches - what you're trying to achieve and for what reason. There is no description and we can only guess. I guess that you're trying to make rpmlint work on new Koji builds and Bodhi updates. While I understand the idea, we're trying to get rid of testing Bodhi updates as much as possible, and we're trying to aggregate the per-Koji-build results in Bodhi interface (not yet implemented). So I don't think we're even interested in this change. Or maybe I just don't understand it. Care to explain your motivation?

It would be easier if you posted patches for already existing tickets, because that's something we already agreed we want to fix, instead of considering some changes we're not sure we even want.

Also please refer to http://nvie.com/posts/a-successful-git-branching-model/ for the git branch naming. Should you decide to use the whole gitflow process, I suggest https://github.com/nvie/gitflow
Also, if there are any tasks related to the diff (or diffs in general), please make sure to use the "Edit Maniphest Tasks" button in the upper-right section of the Differential page to link the respective Task(s) - this will make reviews easier, as we'll be able to instantly see what problem are you trying to tackle.
Thank you!

lnie added a comment.Jul 20 2016, 3:03 AM

I am sorry, but this is absurd - the code, to my knowledge, does not and can not work, at least not without related libtaskotron patches that actually allow for it >to work.

yeah,it is very obviously,like even fool will know, that this diff can't work without libtaskotron's support,and that's exactly the main reason why I think it's not so NOT-OK for me to update the libtaskotron part today.The thing is when I was submitting the libtaskotron part yesterday I found it takes much longer than I thought.The very pretty but annoying girl ,who shares my car with me everyday, kept calling me and yelling :it is reported that a rainstorm is coming ,you know the weather is very bad ,and that made it very hard for me to keep all of my mind on the work(I don't want to say harsh words to her,because she has asked her designer friend help me decorate my house,if anyone wonder). I don't like to submit a diff with lint warns ,so finally I decided to submit another diff today and to be honest,I really felt guilty when I left my seat at 6:25 pm. If there must be a person to be blamed,I hope you can take her as the person,though I know I'm the exact one: )

Have you even tried running it?

I will test a single diff at least 3 times before I submit it,please trust me.

Differentiating between koji build and bodhi update in the reporting code merely based on a presence of a . in the string is _probably_ OK as we are using >update-id's, but I'm not sure that it's foolproof - @kparal, is it?

It seems to be the rule that update id won't consist of even a single .I have gotten the confirmation from a local colleague at lunch.If I'm wrong,would you mind tell me?: )

lnie added a comment.Jul 20 2016, 3:22 AM
In D939#17644, @kparal wrote:

Lili, you need to start by explaining why you're posting patches - what you're trying to achieve and for what reason. There is no description and we can only >guess.

It's really my fault,I thought the description says clearly that this diff want to validate bodhi_id as the item_type for task-rpmlint.

I guess that you're trying to make rpmlint work on new Koji builds and Bodhi updates. While I understand the idea, we're trying to get rid of testing Bodhi >updates as much as possible, and we're trying to aggregate the per-Koji-build results in Bodhi interface (not yet implemented). So I don't think we're even >interested in this change. Or maybe I just don't understand it. Care to explain your motivation?

Exactly: )I found bodhi_directive is unused,then I thought that maybe some users tend to use update id ,and then I asked 3 developers
whether they will use update id for rpmlint.I got two YES,so I have worked on this.It has taken me several hours,I should ask you directly,sigh...

It would be easier if you posted patches for already existing tickets, because that's something we already agreed we want to fix, instead of considering some >changes we're not sure we even want.

Can't agree more,after all of this.I find I'd better try to make contributions on already existing tickets, but not come up with an idea when I look into the code and then submit the diff directly.

lnie added a comment.Jul 20 2016, 3:26 AM

Also please refer to http://nvie.com/posts/a-successful-git-branching-model/ for the git branch naming. Should you decide to use the whole gitflow process, I >suggest https://github.com/nvie/gitflow
Also, if there are any tasks related to the diff (or diffs in general), please make sure to use the "Edit Maniphest Tasks" button in the upper-right section of the >Differential page to link the respective Task(s) - this will make reviews easier, as we'll be able to instantly see what problem are you trying to tackle.

Thanks for your info,I will keep it on mind.

In D939#17680, @lnie wrote:

It's really my fault,I thought the description says clearly that this diff want to validate bodhi_id as the item_type for task-rpmlint.

Yes, I read that, but I didn't understand it. You need to be more verbose. You're often using words that don't make much sense to me in the context (like validate here), so please try to describe your intention in detail, so that we can understand what's going on.

Exactly: )I found bodhi_directive is unused,then I thought that maybe some users tend to use update id ,and then I asked 3 developers
whether they will use update id for rpmlint.I got two YES,so I have worked on this.It has taken me several hours,I should ask you directly,sigh...

I see. The point is that Taskotron tasks are primarily intended to be run in an automated fashion. Of course when creating them or debugging them you need to be able to run them manually, but it's not the primary use case. I don't think it makes much sense for people to manually run task-rpmlint on their machines, using Bodhi IDs and using libtaskotron as the runner. It's much easier for them to run rpmlint directly on their local RPMs, or once they built it in Koji, simply seeing the results in ResultsDB or later in Bodhi (not yet implemented, but almost all pieces are in place and there's no need for Bodhi ID support in tasks). So this use case doesn't really make sense to me.

By the way, is there a reason why you format some of your replies as code (monospaced text)? You should see a preview before submitting your reply, and you can always edit your reply. If you fix your formatting, it will be easier to read. Also, this is very much nit-picking, but could you start adding spaces after commas and fullstops? It would again make it easier to read your replies :) Thanks.

Can't agree more,after all of this.I find I'd better try to make contributions on already existing tickets, but not come up with an idea when I look into the code and then submit the diff directly.

OK, you should see Action -> Abandon Revision. Please do that and this revision will be closed. Sorry for rejecting it.

lnie added a comment.Jul 22 2016, 6:59 AM

Yes, I read that, but I didn't understand it. You need to be more verbose. You're often using words that don't make much sense to me in the context (like >validate here), so please try to describe your intention in detail, so that we can understand what's going on.

By saying validate I mean adding the support for something,ie, we won't get an error because the input data is not supported or is invalid.Obviously,I should pay more attention on using words: )

I see. The point is that Taskotron tasks are primarily intended to be run in an automated fashion. Of course when creating them or debugging them you need to >be able to run them manually, but it's not the primary use case. I don't think it makes much sense for people to manually run task-rpmlint on their machines, >using Bodhi IDs and using libtaskotron as the runner. It's much easier for them to run rpmlint directly on their local RPMs, or once they built it in Koji, simply >seeing the results in ResultsDB or later in Bodhi (not yet implemented, but almost all pieces are in place and there's no need for Bodhi ID support in tasks). So >this use case doesn't really make sense to me.

I was thinking that they have to run koji download-build to download the RPMS and then run rpmlint on them when the downloading is finished,if they want to run rpmlint themselves on RPMS which is already in koji or bodhi every time .With libtaskotron,they only need install it once,and run a simple command,but not 'run a command,then wait ,then run another command'.But your words do make sense for me ,and have made me suddenly understand the real meaning when one of them said if only bodhi could do that for us.They mean the rpmlint logs being put into bodhi by Automation tools,which is libtaskotron:)

You should see a preview before submitting your reply, and you >can always edit your reply. If you fix your formatting, it will be easier to read. Also, this is very >much nit-picking, but could you start adding spaces after commas and fullstops? It would again make it easier to read your replies :) Thanks.

No,it's not nit-picking,it's just you are trying to help me to be professional.

OK, you should see Action -> Abandon Revision. Please do that and this revision will be closed. Sorry for rejecting it.

Don't be,I should have obtained the sense which diff is likely to be welcomed by libtaskotron ,instead of wasting our time:)

lnie abandoned this revision.Jul 22 2016, 6:59 AM

I was thinking that they have to run koji download-build to download the RPMS and then run rpmlint on them when the downloading is finished,if they want to run rpmlint themselves on RPMS which is already in koji or bodhi every time .With libtaskotron,they only need install it once,and run a simple command,but not 'run a command,then wait ,then run another command'.

Those package maintainers you spoke to probably want to do something like this (simpler than executing taskotron):

$ koji download-build BUILD && rpmlint ./*.rpm

But your words do make sense for me ,and have made me suddenly understand the real meaning when one of them said if only bodhi could do that for us.They mean the rpmlint logs being put into bodhi by Automation tools,which is libtaskotron:)

We will do that for them, once the feature is ready. However, it would be much better if they checked rpmlint logs before they submit it into Bodhi. Submitting it into Bodhi means it's already get deployed to some people's machines. If they want to check rpmlint output, they should do it immediately after building (either locally, or in Koji).