I don't think it causes any problems. I don't see the reason why the whole commit should be reverted. If something then perhaps deprecation warning could be removed. I'd still prefer communuty-templates using macros instead of internal objects.

--
Marek

Odesláno pomocí AquaMail pro Android
http://www.aqua-mail.com


Dne 13. ledna 2017 18:27:13 "Sean O'Keeffe" <sokee...@redhat.com> napsal:

Maybe the best thing to do for now it to revert it and send a PR to the RFC
repo for a proper discussion?

On Fri, Jan 13, 2017 at 2:26 PM, Daniel Lobato Garcia <elobat...@gmail.com>
wrote:

On 01/12, Marek Hulán wrote:
 > >
> > > I strongly disagree that this does not have big benefits. Using
internal
> > > Foreman objects in templates is a bad practice. It blocks us from
> > > improving
> > > our code. Therefore it's very important to build a DSL that users
can use
> > > in templates and keep that compatible. We can then later change the
> > > implementation of host_param_true method without any templates
changing.
> > > Think
> > > of this as a templating API.
> > >
> > > Another less significant benefit is that for plugins it's easier to
> > > wrap/alias
> > > the template method rather than manipulating something that's used
> > > internally in @host. Still not ideal but that should be solved by
> > > https://github.com/ theforeman/foreman/pull/3701
> > >
> > > Of course it will require users to migrate to new template helpers
which
> > > is
> > > why we move there slowly and hopefully with proper deprecations. I
was
> > > hoping to do the update for community-templates since it's very easy
> > > migration.
> > >
> > > If you think it's too complicated for users we could provide rake
task or
> > > migration. But please don't revert this.
> >
> > Yes, if you provided a migration it would be much better.  That doesn't
> > really solve the problem with people using the foreman_templates plugin
> > who will have those changes reverted, but I guess it's better than
nothing.
> >
> > There's still dozens of other things allowed for @host in the Jail that
> > aren't covered by these macros.   What's the plan for those?
>
> Whenever we have a chance, we should move from internal objects to
macros. The
> more macros we have the higher the chance is that we can keep templates
> compatible.

Sorry but I oppose this. Some internal objects are quite stable, in
fact people rely upon them successfully - but we break the compatibility
for apparent advantages that IMO are not worth the hassle.

@host.operatingsystem, @host.architecture, etc... and @host.params, are
very stable objects that can safely be called from templates and expect
that will work for a long time. Not only our users do it. We also relied
upon these internal objects for years.

The first community templates PRs which are 4 years old used
@host.params, @host.operatingsystem, etc.. Those are stable enough to
not warrant writing a macro for them

What we did on #16740 is basically renaming things around and deprecating
params for macros that don't do anything but calling @host.params
themselves.

I would argue for the opposite way of thinking. When we change how
these internal APIs work, we should deprecate them. If these APIs
remain stable, don't change anything as it provides no value, and brings
headaches and wastes time. Time wasted on writing this thread, on the
people who write and review PRs in the project and associated ones
(templates, the migration to new macros, fixing anything that might break,
and adding more LOC which complicate our codebase).

>
> > I still think this provides more headaches than any benefits.
>
> I hope that following helps
> * https://github.com/theforeman/community-templates/pull/343
> * https://github.com/theforeman/foreman/pull/4187
> * https://gist.github.com/ares/5435226ef0317613535101765404d3f5
>
> --
> Marek
>
> >
> >
> > - Stephen
> >
> > > --
> > > Marek
> > >
> > > --
> > > You received this message because you are subscribed to the Google
Groups
> > > "foreman-dev" group.
> > > To unsubscribe from this group and stop receiving emails from it,
send an
> > > email to foreman-dev+unsubscr...@googlegroups.com.
> > > For more options, visit https://groups.google.com/d/optout.
>
> --
> You received this message because you are subscribed to the Google
Groups "foreman-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send
an email to foreman-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

--
Daniel Lobato Garcia

@dLobatog
blog.daniellobato.me
daniellobato.me

GPG: http://keys.gnupg.net/pks/lookup?op=get&search=0x7A92D6DD38D6DE30
Keybase: https://keybase.io/elobato

--
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups "foreman-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to