Pull request triage - 2014-06-04

[Ethan Brown]

Joined: Andy, Rob, JoshP, Josh, Adrien, Ethan, BLaMe, Charlie, Kylo, Peter, FelixF

Candidates for merging:

  - 2577: (PUP-2356) Added some extra yumrepo options 

      - Charlie Reviewing, needs to be rebased

  - 2736: (PUP-2454) Don't purge system users on OpenBSD

      - Formerly 2547, currently failing TravisCI build

  - 2717 (maint): Consolidate case statement in /pson/pure/generator.rb

      - Josh to take a look and merge

  - 2719 (PUP-2506): Error when evaluating #type in Puppet::Error message interpolation for Puppet::Resource::Ral

      - Kylo to review / merge

  - 2723 (maint): Move duplicate code in /lib/puppet/provider/service/freebsd.rb

      - Peter to review / merge

  - 2730 (maint): remove dead services code from puppet.rb

      - Andy to merge during triage

  - 2731 (maint): remove dead name from puppet.rb

      - Andy to merge during triage

  - 2732 (maint): remove genmanifest from puppet.rb

      - Andy to merge during triage

  - 683 (FACT-484): check dmi sysfs file for readability instead of just existence

      - Adrien to merge, should be on facter-2 branch

Held over:

Puppet:

2638 (PUP-2520): implements 'manual' environment directories

  - JoshP to contact contributor on PR

  - commits should reference PUP-2520 instead of 2520

  - dubious value, and other higher priority work on environments ongoing

  - will consider pulling into a future sprint, but no time presently, most

    likely landing in Puppet 4

2706 (docs): Edit header of metaparameter reference

  - Andy to merge during triage

2737 (docs): Use git checkout -b

  - Charlie to update commit msg and merge

2707 (PUP-2635): Use generate instead of eval_generate to purge ssh keys

  - Andy to investigate prioritizers and comments made on PR

  - requires additional review before considering merge

2709 (PUP-2701): Add PMT build metadata.json tests

  - Joshua Partlow to verify these on a few platforms before merging

2715 (PUP-2705): External facts pluginsync not preserving source permissions

  - Josh to follow up with contributor in an effort to further evaluate the gameplan

2716 (PUP-???): zypper should always be the default package provider for SUSE osfamily

  - Needs a PUP-ticket

  - Those present are not totally sure if zypper is the appropriate default and whether

    or not this could break existing Puppet installs.  Adrien is contacting colleague

    at Attachmate / Novell?

  - Rug was the previous default, but modern systems appear to be defaulting to zypper

2717 (maint): Consolidate case statement in /pson/pure/generator.rb

  - Josh to take a look and merge

2719 (PUP-2506): Error when evaluating #type in Puppet::Error message interpolation for Puppet::Resource::Ral

  - Kylo to review / merge

2723 (maint): Move duplicate code in /lib/puppet/provider/service/freebsd.rb

  - Peter to review / merge

2725 (maint): Remove code duplication in lib/puppet/type/yumrepo.rb

  - Adrien to comment / likely close - code is more difficult to read / maintain

2734 (PUP-2579): Improve regular expression for options of ssh_authorized_keys

  - Josh to review, noticed "type" is now a reserved word and will file additional ticket for that

  - Follow up with contributor about adding tests, which are non-trivial

  - Need CLA from contributor

2735 (maint): Remove property shadowing

  - Adrien has determined this code was never used and is completely dead weight

  - Adrien to fixup code by removing initializer

2738 (maint): Reduce redunandcy in Puppet::Util::Posix

  - requires some minor tweaks to naming - ie 'the_thing' should be 'group_or_user' 

2468 (PUP-2079): Add mechanism to allow template files to be copied without getting parsed

  - This is a modules/forge team, Adrien to ping everybody remotely related to the forge

  - The changes that were blocking this have landed. Kylo pinging pvande again.

  - Need to ping pvande again to verify / merge

  - Still no response, will wait another week

  * Still no response as of 6/4

2704: (PUP-2569) (maint) Return last serial from SSL inventory

  - Andy to log a ticket, continue discussion on ticket, e.g. revoke all certificates for a specified name

  - PR needs to be updated with issue # in commit, revoke all certificates with a given name, preserve existing API

  - Kylo to update pull request

  * formerly 2501, still open as of 6/4

2577: (PUP-2356) Added some extra yumrepo options

  - Charlie to follow up with contributor on test failures

  - Otherwise, Adrien and Charlie give this the +1

  - PR spec tests updated, pulling into 05/21 - 05/28 sprint

  * Needs to be rebased as of 6/4

2557: (PUP-2578) Unbreak OpenBSD services from packages which set local flags

  - Needs tests, Rob to comment

  - Our current sprint is pretty full, won't be able to pull into this week's sprint

  - Sprint is still pretty full, deferring for another week

  * failed TravisCI build as of 6/4

2558: (PUP-1069) upgradeable OpenBSD package provider

  - Second PR to adding this functionality

  - Adrien to comment in the PR that we want this but will need to defer another week (2x)

  * still open as of 6/4

2736: (PUP-2454) Don't purge system users on OpenBSD

  - Current behavior is dangerous on OpenBSD & Debian, this would provide incremental improvement

  - Pull request updated for Debian but Debian system UIDs are 100 - 999, comment added

  - Peter worked on during the PR Triage. Jasper needs to fix up.

  * formerly 2547, TravisCI failing as of 6/4

2605: (PUP-1381) Read all crontabs

  - Felix to update PR with JIRA ticket for 4.0

  - Felix to file separate deprecation_warning ticket for 3.7

  - Felix to update pull request with points from triage discussion

  * still open as of 6/4

2666: (maint) Resolve some documentation TODOs in Puppet::Type

  - There are concerns about #uniqueness_key being public API, and composite

    namevars are not well supported. We need to decide if we want to really

    support those.

  * still open as of 6/4, Felix cleaned up PR on 6/2

--

--

Ethan Brown

et...@puppetlabs.com

Software Engineer

Join us at PuppetConf 2014, September 20-24 in San Francisco

Register by June 5th to take advantage of the Early Adopter discount —save $349!

[Robin Bowes]

On Wed, 2014-06-04 at 14:12 -0700, Ethan Brown wrote:

>
> 2468 (PUP-2079): Add mechanism to allow template files to be copied
> without getting parsed
> - This is a modules/forge team, Adrien to ping everybody remotely
> related to the forge
> - The changes that were blocking this have landed. Kylo pinging
> pvande again.
> - Need to ping pvande again to verify / merge
> - Still no response, will wait another week
> * Still no response as of 6/4

I'm rather mystified why this is continually put back.

What's the hold up?

R.

[Rob Reynolds]

In this case we are waiting on verification from another team and they made changes that conflicted with this change as well. 

 

R.

--
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/1401999827.13490.10.camel%40hero.robinbowes.com.

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

--

Rob Reynolds

Developer, Puppet Labs

[Brice Figureau]

Hi,

On Wed, 2014-06-04 at 14:12 -0700, Ethan Brown wrote:

>

> 2638 (PUP-2520): implements 'manual' environment directories
> - JoshP to contact contributor on PR
> - commits should reference PUP-2520 instead of 2520

My bad, I should have read the contributing doc before submitting. I'll
amend the commits.

> - dubious value, and other higher priority work on environments
> ongoing

I can't help thinking I fell in a trap. Why open a ticket, asking for a
contribution on the mailing list, if nobody thinks it's a good idea. I
spent a bit of time contributing this code I could have spent on other
things :(

> - will consider pulling into a future sprint, but no time presently,
> most
> likely landing in Puppet 4

[snip]

> 2704: (PUP-2569) (maint) Return last serial from SSL inventory
> - Andy to log a ticket, continue discussion on ticket, e.g. revoke
> all certificates for a specified name
>
> - PR needs to be updated with issue # in commit, revoke all
> certificates with a given name, preserve existing API

I'm not sure I'll get it. To my knowledge the PR as the issue # in the
commit, a correct title, does its job and does preserve the existing
API.
Can someone enlighten me about the problem?

> - Kylo to update pull request
> * formerly 2501, still open as of 6/4

Thanks,
--
Brice Figureau
Follow the latest Puppet Community evolutions on www.planetpuppet.org!

[Felix Frank]

On 06/06/2014 09:31 AM, Brice Figureau wrote:
>> - dubious value, and other higher priority work on environments
>> > ongoing
> I can't help thinking I fell in a trap. Why open a ticket, asking for a
> contribution on the mailing list, if nobody thinks it's a good idea. I
> spent a bit of time contributing this code I could have spent on other
> things :(
>

For what it's worth - the discussion rather concerned itself with the
issues with *any* cache invalidation that is no graceful webserver
restart. Favoring your implementation over the timeout based approach is
definitely on the table (and would be my personal preference). It'll
definitely be a load of work to review and merge it, obviously.

But please also bear in mind that as contributors, we always carry the
risk that our designs don't pass by all gatekeepers. Even if one
developer asks for an implementation, there is always a consensus to be
reached, and this will not always be possible before looking at PoC
code, or will just not happen in time because it's a logistic
impossibility that the whole team monitors all threads and tickets.

TL;DR I feel your pain and hope this works out, but some times things won't.

Cheers,
Felix

[Ethan Brown]

Brice - 

On the held over issues, we didn't get a chance to discuss some of the ones from the prior week.  All the *'s were quick updates, based on my scanning of those open PRs.  

<inline />

In some cases, since it was a quick pass, some errant comments from the previous week were held over, which is the case here.  For this one in particular, a completely new PR was opened that fixed most of the problems with the old one.  However, we didn't get a chance to fully discuss during the triage this week, so it will be addressed in the next triage.  FWIW, an updated version of the PR was pushed up on Wednesday AM.

 

>   - Kylo to update pull request
>   * formerly 2501, still open as of 6/4

Apologies for any confusion this caused.  I should have segregated the issues from last week that we weren't able to address this past Wednesday.