Agreed Otto, the challenge is that essentially each change cuts across dependencies in every component. I could break it down into the changes for making SSO work, and the changes for making it install, and the changes for making full-dev work, but that would mean violating our policy that testing should be done for each PR on full dev, hence the one PR one unit approach. Does that work, or do we want to review on the basis of a series of untestable bits, and then a final working build PR that pulls it together?
Simon On 12 July 2018 at 16:00, Otto Fowler <ottobackwa...@gmail.com> wrote: > Our policy in the past on such things is to require that they are broken > into small reviewable chunks on a feature branch, even if the end to end > working version was more ‘usable’. > > > > On July 12, 2018 at 10:51:30, Simon Elliston Ball ( > si...@simonellistonball.com) wrote: > > I've been doing some work on getting the Metron UIs and REST layers to > work > with Apache KnoxSSO, and LDAP authentication, to remove the need to store > passwords in MySQL, allow AD integration, secure up our authentication > points. I'm also working in a Knox service to allow the gateway to provide > full SSL for the interfaces and avoid all the proxying and CORS things we > have to worry about. > > This has ended up being a pretty chunky piece of work which involves very > significant changes to the UIs, REST layer, and introduces Knox to the > blueprint, as well as messing with the full-dev build scripts, and adding > ansible roles. > > As such, in-order to make it a bit more reviewable, would it be better to > contribute it to a feature branch? It could arguably be broken into a > series of PRs, but at least some parts of full dev would be broken between > most of the logical steps, since it's all kinda co-dependent, so it's > easier to look at as a unit. > > Simon > > -- -- simon elliston ball @sireb
