Committer question: In which branch should new facts go

[Stefan Schulte]

Hello,

I haven't merged any code since the new semver scheme and I am currently
reviewing https://github.com/puppetlabs/facter/pull/560
which introduces a new uuid fact for blockdevices. My question is now:
In which branch should this code be merged?

From my understanding it should go in the next minor version because a
new fact falls under the definition of "add functionality in a
backwards-compatible manner"

Now https://github.com/puppetlabs/facter/blob/master/COMMITTERS.md talks
about branches for each minor version, so I'd guessed there is a "1.7.x"
(patches only), a "1.8.x" (patches and new features) and a "2.0.x" (for
incompatible changes) branch but when I run git branch I can only see


remotes/upstream/1.6.x
remotes/upstream/facter-2
remotes/upstream/gh-pages
remotes/upstream/master
remotes/upstream/stable

Can someone please explain the purpose of each branch and where to merge
the different changesets?

-Stefan

[Kylo Ginsberg]

On Sun, Feb 16, 2014 at 5:09 AM, Stefan Schulte <stefan....@taunusstein.net> wrote:

In which branch should this code be merged?

Please target the 'facter-2' branch. See this puppet-dev thread for some background:

https://groups.google.com/forum/#!msg/puppet-dev/Q24GLe6s1_4/hPwHgvVhnpIJ

Now https://github.com/puppetlabs/facter/blob/master/COMMITTERS.md talks

Oops, that doc was never updated to reflect the above. Thanks for pointing that out!  Pull coming ...

Kylo

[Stefan Schulte]

On 16.02.2014 16:43, Kylo Ginsberg wrote:
> Please target the 'facter-2' branch. See this puppet-dev thread for some
> background:
>
> https://groups.google.com/forum/#!msg/puppet-dev/Q24GLe6s1_4/hPwHgvVhnpIJ
>
>
> Now https://github.com/puppetlabs/facter/blob/master/COMMITTERS.md talks
>
>
> Oops, that doc was never updated to reflect the above. Thanks for
> pointing that out! Pull coming ...
>
> Kylo
>

Hi Kylo,

thanks for the quick response!

-Stefan

[Adrien Thebo]

I should have responded sooner to this thread, but now that the horse has clearly left the barn I might as well shut the gate.

We've been working on getting Facter 2 ready for release for about two months now, and right now we're right on the cusp of shipping an RC. The goal for the initial release is to make the necessary changes to the internal API to permit structured data and minimize the amount of changes otherwise. The hope is that we can finally ship Facter 2, the initial release will be stable and will be as free of surprises possible, and then we can work on releasing new facts that can now be composed of structured data. The approach for Facter 2 is outlined in https://groups.google.com/forum/#!topic/puppet-dev/Q24GLe6s1_4 .

The partition UUID fact pull request has been outstanding for a while, and that's entirely on me. I've been holding off on merging new pull requests because 1) we will not be releasing a lot of new facts in Facter 2 and 2) master is in limbo so merging new facts into master won't achieve very much. Facter 2.1 will pull in a number of new facts and start taking advantage of structured facts and richer data types, but I do want to push those sorts of changes down the road so that we can focus on just getting an uneventful Facter 2 out the door.

The partition UUID fact is especially problematic because it's a fantastic candidate to use as part of a structured fact for partition information. If we ship the flat fact version in Facter 2 that means that we're committed to those facts for the lifespan of Facter 2, and when we start building up structured facts then we'll be duplicating this information. I would rather not ship a lot of new facts with Facter 2.0, and I would prefer that we wait a bit and support this correctly rather than ship it right now.

Stefan, how adamant are you about including these facts for Facter 2.0? Is there any way that we can postpone this till Facter 2.1?

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/53010266.2080207%40taunusstein.net.

For more options, visit https://groups.google.com/groups/opt_out.

--

Adrien Thebo | Puppet Labs

[Stefan Schulte]

Hi Adrien,


On 18.02.2014 18:50, Adrien Thebo wrote:> I should have responded sooner

to this thread, but now that the horse
> has clearly left the barn I might as well shut the gate.
>
> We've been working on getting Facter 2 ready for release for about two
> months now, and right now we're right on the cusp of shipping an RC. The
> goal for the initial release is to make the necessary changes to the
> internal API to permit structured data and minimize the amount of
> changes otherwise. The hope is that we can finally ship Facter 2, the
> initial release will be stable and will be as free of surprises
> possible, and then we can work on releasing new facts that can now be
> composed of structured data.

to be honest the role of the master and facter-2 branch are still not
clear to me. What I've did so far

1) The original pull request was based on master, so I've rebased on
facter-2
2) I've already merged the pull request in facter-2 yesterday (sorry for
that, I did not notice the "The reload to facter 2" thread and your
answer comes to late for me now)
3) I've already merged facter-2 into master

If I did understand you correctly, I'd rather have merged it directly
into master and hoped someone would backport it to facter-2, once facter
2.0 is out the door?

> The partition UUID fact is especially problematic because it's a
> fantastic candidate to use as part of a structured fact for partition
> information.

You are right that the new uuid facts represent structured data in a
flat, unstructured way, but that is also true for all the other
blockdevice facts. Are there actually any structured facts in the
facter-2 codebase? When I run `facter` on the `facter-2` branch, I do
not see a single array or hash, just strings. Having a few examples
might be helpful to actually write a proper facter2 fact (or should all
current facts stay-as-is and only new facts should make use of the new
API? Personally I'd be confused if some blockdevice facts where
unstructured while others in the same realm were structured)

> Stefan, how adamant are you about including these facts for Facter 2.0?
> Is there any way that we can postpone this till Facter 2.1?

Personally I do not have any use for that fact. I just noticed the pull
request did not receive any reaction for a long time and thought I might
help out. So if you plan on reverting the change to have a clean
facter-2 codebase, that's better answered by Chris Portman than myself
(merged pull request for reference:
https://github.com/puppetlabs/facter/pull/560)

-Stefan

[Adrien Thebo]

On Tue, Feb 18, 2014 at 2:52 PM, Stefan Schulte <stefan....@taunusstein.net> wrote:

Hi Adrien,


On 18.02.2014 18:50, Adrien Thebo wrote:> I should have responded sooner

to this thread, but now that the horse
> has clearly left the barn I might as well shut the gate.
>
> We've been working on getting Facter 2 ready for release for about two
> months now, and right now we're right on the cusp of shipping an RC. The
> goal for the initial release is to make the necessary changes to the
> internal API to permit structured data and minimize the amount of
> changes otherwise. The hope is that we can finally ship Facter 2, the
> initial release will be stable and will be as free of surprises
> possible, and then we can work on releasing new facts that can now be
> composed of structured data.

to be honest the role of the master and facter-2 branch are still not
clear to me. What I've did so far

1) The original pull request was based on master, so I've rebased on
facter-2
2) I've already merged the pull request in facter-2 yesterday (sorry for
that, I did not notice the "The reload to facter 2" thread and your
answer comes to late for me now)
3) I've already merged facter-2 into master

If I did understand you correctly, I'd rather have merged it directly
into master and hoped someone would backport it to facter-2, once facter
2.0 is out the door?

The blockdevices fact (or set of facts) is one of the facts that's seen a lot of divergence between the facter-2 and master branches. In cases where there's a lot of divergence I've had to cherry-pick the relevant commits back onto facter-2 to make the merge-ups less terrible, and in other cases I've foregone making some changes to avoid making some really messy merges.

Right now I'm asking people to completely hold off on merges for a little while longer, because are very close to going into RC for Facter 2. When I say "very close" I mean "tomorrow, with any luck," so this isn't a far off pipe dream - we're talking by Friday at the outside. Because we're so close I want to keep things as stable as we can.

> The partition UUID fact is especially problematic because it's a
> fantastic candidate to use as part of a structured fact for partition
> information.

You are right that the new uuid facts represent structured data in a
flat, unstructured way, but that is also true for all the other
blockdevice facts. Are there actually any structured facts in the
facter-2 codebase? When I run `facter` on the `facter-2` branch, I do
not see a single array or hash, just strings. Having a few examples
might be helpful to actually write a proper facter2 fact (or should all
current facts stay-as-is and only new facts should make use of the new
API? Personally I'd be confused if some blockdevice facts where
unstructured while others in the same realm were structured)

We've chosen to try to reduce the amount of possible churn and backwards incompatibilities in Facter 2, so Facter 2 doesn't introduce any new structured facts. As part of the Facter 2 release we'll also provide documentation on how to create structured facts, and for that matter I've been working with our docs team to start getting those written. However, the short of it is that this:

Facter.add(:structured) do

  setcode do

    {"hello" => "world!"}

  end

end

Will do exactly what you would expect.

Since Facter 2 has been in limbo so long the real focus of the 2.0 release is to get structured fact support out into the world, minimize the amount of changes, and generally get things back in motion. For Facter 2.1 I expect that we'll provide new implementations for block devices, filesystem information, network devices, and so forth that will take advantage of the new structured fact functionality.

> Stefan, how adamant are you about including these facts for Facter 2.0?
> Is there any way that we can postpone this till Facter 2.1?

Personally I do not have any use for that fact. I just noticed the pull
request did not receive any reaction for a long time and thought I might
help out. So if you plan on reverting the change to have a clean
facter-2 codebase, that's better answered by Chris Portman than myself
(merged pull request for reference:
https://github.com/puppetlabs/facter/pull/560)

I appreciate that you took the time to do this. :) I've done a poor job of communicating my expectations about the Facter 2 release and we've let a number of pull requests silently sit, and that's entirely on me. I'll update that pull request and ask Chris what he thinks about this situation.

 

-Stefan

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/5303E411.2030107%40taunusstein.net.

For more options, visit https://groups.google.com/groups/opt_out.