← Back to team overview

fuel-dev team mailing list archive

Re: Please don't turn Nailgun to a pile of botch

 

+ fuel-dev
On Feb 19, 2014 9:26 PM, "Nikolay Markov" <nmarkov@xxxxxxxxxxxx> wrote:

> Colleagues,
>
> I wanted to draw your attention to the fact that all we're working on
> the same code in Nailgun, and this code is at least trying to follow
> some practices which we are discussing together from time to time and
> even have some documentation on it. But at the present moment, just
> before release, there are a lot of "patches" which mask the gaps in
> our code and break the backroom code style. That's a completely normal
> situation for facing the release. I just really want you not to turn
> our code into a mess of garbage. Speaking that, I look at several
> things:
>
> 1) Bunch of "helper" methods in TaskHelper object, which should be
> moved to corresponding object. There should NOT be any TaskHelper
> object in the future at all. Some may argue that there are some
> methods which are some basic snippets without exact business logic and
> they should be moved somewhere to "utils" module. I know exactly one
> helper which deserves that - recursive dict traversing. If anyone
> knows some more - don't hesitate to discuss it.
>
> 2) Even though we have such an object (see 1), some still prefer
> adding "helper functions" somewhere in global scope. Others prefer
> adding some helper methods to web handlers, right near GET(), PUT()
> and POST(). Come on, guys, we're switching to objects and all businell
> logic should be there. There is no any reason to have any non-web
> related code in handlers, they just implement our REST and that's all.
>
> 3) For those of you who laughed at "implement our REST" - go and
> submit pull request which will remove this word from anywhere in
> documentation and then implement all handlers you want. But if we are
> still following our REST approach - please, don't make such things, as
> "separate web handler for updating that field somewhere deep in Node's
> json metadata". Like, I heard really a lot of questions on
> implementing separate handler for assigning Node to Cluster - they
> wouldn't even appear if we would follow our REST approach. And this
> handler has nothing to do with it. If somebody is not agree - let's
> discuss it here.
>
> I am not pointing fingers, but please, when you're implementing such a
> thing - at least create corresponding bug/blueprint/note on a piece of
> paper so you won't forget to fix it in the future, just after release.
> The same thing for forgotten comments in some pull requests with "will
> fix in next PR" remark.
>
> I'm going to initiate writing of development documentation after
> release, so everyone will know where to find an answer to "which
> approach to choose here or there". Because now everyone is just
> pulling the blanket, let's find a compromise.
>
> --
> Best regards,
> Nick Markov
>
> --
> You received this message because you are subscribed to the Google Groups
> "fuel-core-team" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to fuel-core-team+unsubscribe@xxxxxxxxxxxx.
> For more options, visit
> https://groups.google.com/a/mirantis.com/groups/opt_out.
>