AshtonKem / Butler

Jenkins/Hudson plugin for Emacs
44 stars 10 forks source link

Add job information retrieval #12

Closed AeroNotix closed 7 years ago

AeroNotix commented 10 years ago

:rotating_light: WIP :rotating_light:

I'm submitting this as a WIP to get feedback and to elicit discussion about the overall architecture of the code.

1) The HTTP callback business has to go. 2) Retrieving the authentication information before each HTTP call is annoying. 3) The global hash map makes for an awkward api (Ref #https://github.com/AshtonKem/Butler/issues/11) 4) Pertaining to 2+3, maybe some kind of proper object (defstruct?) could be used for better access to relevant information?

As for the code in this PR -- please review. I want to provide a better view of the actual job data, opinions welcome there.

Questions:

Should the information query automatically query the last 10 or so jobs (each a new HTTP request!) or not? Is there a way to get this information in one go?

AeroNotix commented 10 years ago

@AshtonKem any help with this? Are you longer supporting Butler?

AshtonKem commented 10 years ago

I am, on and off. Since I don't get to use my own editor at my new job (pair programming), I don't really have as much pressure to improve it on a regular basis. I'll review this PR eventually, but don't expect it to happen super fast.

AshtonKem commented 10 years ago

So, before I start commenting on individual bits, I'm not sure I agree with point 1. I tried earlier in Butler's lifecycle to use direct HTTP gets, but that noticeably locked up Emacs during the duration of the get. So the callback got added as a compromise to make Butler more lively.

2) I would love to use cookies, not sure how though.

3 & 4) Agreed.

AeroNotix commented 10 years ago

When I get some spare cycles this week I'll attack 2,3,4. I'll try to propose something better for 1, too.

AshtonKem commented 10 years ago

Sure thing. Overall it looks fine to me, but I need to reinstall Jenkins (oh the joy of small harddrives) to try it out later tonight or tomorrow. Once you remove hard-coded references to your Jenkins' jobs, we can probably merge this in and then tackle other issues later.

AeroNotix commented 10 years ago

Will do, cheers!