search

feedhenry / rhm

4 stars 6 forks source link

Add getting of templates from RHMAP #20

Closed aidenkeating closed 7 years ago

aidenkeating commented 7 years ago

There is still a lot to do here, I started working on it only getting projects, but it makes sense to instead allow all types of templates to be gotten through this PR just by generalising everything a bit so I've started on that now.

Just wanted to keep this here so I can get some feedback. It's used like this:

rhm get templates projects

which is similar to the fhc style, however maybe something like

rhm get templates --type=project

would make more sense. Then the directory structure could be flattened again.

At the moment it is also only list, not getting an individual one.

@maleck13 would you mind taking a look?

_Edit:_ It also needs tests too.

aidenkeating commented 7 years ago

Oh I see that golint issue, I'll fix that too

aidenkeating commented 7 years ago

@maleck13 I've made quite large changes to this now locally. I'll update this PR with those changes when they're complete. Thanks :)

aidenkeating commented 7 years ago

@maleck13 Is this more like what you were thinking, usage is

rhm get templates --type=[projects,apps,etc.]
aidenkeating commented 7 years ago

@maleck13 Could you take a look, I'm not sure if there's much more to add.

@luigizuccarelli I have fixed that issue now with the leading slash not being added to the url path.

luigizuccarelli commented 7 years ago

@aidenkeating - tested it locally - working now

wtrocki commented 7 years ago

Looks great! Can we merge that as it's blocking couple other commands :)

aidenkeating commented 7 years ago

@maleck13 Is this ok to merge?

Edit: Should all of the values in the JSON response be printed to the console? For example, priority and icon

maleck13 commented 7 years ago

@aidenkeating I think it looks good as it is. Nice work. Only one suggestion for a follow on PR is to cache the results and and a -f flag to allow force downloading them again. Templates don't change that often.

aidenkeating commented 7 years ago

@maleck13 I will remove that line in the test file and merge.

wtrocki commented 7 years ago

cache the results and and a -f flag to allow force downloading them again

@maleck13 I would add that as part for new subcommand implementation.