proposed implementation of distro constants by bobchaos · Pull Request #112 · chef/chef-apply

bobchaos · 2019-11-12T18:37:55Z

Signed-off-by: Marc Chamberland chamberland.marc@gmail.com

Description

In order to support redistributing workstation we need to remove Chef Inc trademarks from chef-apply/chef-run. This PR proposes an implementation based on what is currently accepted for chef/chef as described here: chef/chef#8376

Related Issue

None, but I can open one if needed, either here or VS the workstation repo.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

I have read the CONTRIBUTING document.
I have run the pre-merge tests locally and they pass.
[NA] I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.
All commits have been signed-off for the Developer Certificate of Origin.

tyler-ball

1 question but generally I think this looks good and starightforward. Thanks @bobchaos

tyler-ball · 2019-11-12T21:40:35Z

+    WEBSITE = "https://chef.io".freeze
+
+    # chef-apply's product name
+    APPLY = "Chef Infra Apply".freeze


Hmm, hadn't thought about this one. Seems right to me, but I could be persuaded that Chef Apply is better. I think Chef Infra Apply is more appropriate since it is a subset of the chef command. @chef/chef-workstation-owners what do y'all think?

I would personally check with marketing/docs teams. I vote as is Chef Infra Apply. 🥇

I've sent the question over to marketing and will post back with any updates.

Any news on this? I'm going through workstation dependencies to update Cinc's backlog.

This turns out to be a little tricky, because 'chef-apply' (chef-run) isn't a product itself. We're continuing to have discussions around this internally, but I don't yet have a time frame for when we'll have a definitive answer.

It looks like we're not referencing this in the chef_apply repository - would it make sense to remove this const for now, or would that cause problems for Cinc's implementation?

If it's not a word we need to worry about then it could certainly be removed. I'll try to book some time to fix up this PR over the next two weeks, we can at least see how it looks. Only worry I can think of is it's going to look a bit weird to have a single tool in workstation still be called chef-* where everything else is cinc-*, but we can address that by patching + wrappers until a definitive answer is found

Can we go with "chef-run"? It'll eliminate confusion around chef-apply/chef-run and side-steps the whole "isn't a product" problem.

tyler-ball · 2019-11-12T21:46:49Z

      UI::Terminal.output(T.error.bad_config_file(e.path))
    rescue ConfigPathNotProvided
-      UI::Terminal.output(T.error.missing_config_path)
+      UI::Terminal.output(T.error.missing_config_path(D::RUNEXEC, D::DK))


This is definitely appropriate looking at the error message, but internally we have a ticket to change this from .chefdk to .chef-workstation. We'll come back and clean this up 😄

afiune · 2019-11-13T01:54:27Z

    class InstallChef < Base
      class MinimumChefVersion

+        D = ChefApply::Dist


I feel this is a bit obscure but, I also don't like typing that much so, it is fine I guess. 😄

Also, other parts of the codebase do it, so 👍

all the ChefApply::Dist::SOMEREALLYLONGSTRINGS were making my eyes bleed o.O

bobchaos · 2020-01-28T22:06:23Z

I'll give this thing a quick rebase. Thanks @afiune !

bobchaos · 2020-04-26T13:51:58Z

Just going over my pending PRs in preparation of May 1st (next week! :O ). Anything missing to get this merged?

ramereth · 2020-06-09T23:33:12Z

@marcparadise @tyler-ball I rebased this the other day. I was hoping you could take another look at this so we can start using this. Thanks!

marcparadise

Just one minor change, setting "Chef Infra Apply" to "chef-run". We'll just call it what the command name is for now, so that the open product question doesn't block this any longer.

Also, please run rake style:autocorrect to clear style warning so that the lint tests pass.

If the Windows failure continues on next build, I'll take a look and see what we need to fix in deps.

Signed-off-by: Marc Chamberland <chamberland.marc@gmail.com>

Signed-off-by: Lance Albertson <lance@osuosl.org>

tyler-ball · 2020-07-08T19:03:15Z

+      https://%4/chef-workstation/stable

  version:
    description: Show the current version of Chef Run.


Suggested change

description: Show the current version of Chef Run.

description: Show the current version of %1.

tyler-ball · 2020-07-08T19:03:46Z

      Download the latest here:

-      https://downloads.chef.io/chef-workstation/stable
+      https://%4/chef-workstation/stable


Suggested change

https://%4/chef-workstation/stable

https://%4/%5/stable

tyler-ball · 2020-07-08T19:04:50Z

+      UI::Terminal.output(T.error.missing_config_path(ChefApply::Dist::RUNEXEC, ChefApply::Dist::DK))
    rescue UnsupportedInstallation
-      UI::Terminal.output(T.error.unsupported_installation)
+      UI::Terminal.output(T.error.unsupported_installation(ChefApply::Dist::RUNEXEC, ChefApply::Dist::DK, ChefApply::Dist::WORKSTATION, ChefApply::Dist::DOWNLOADS_URL))


Suggested change

UI::Terminal.output(T.error.unsupported_installation(ChefApply::Dist::RUNEXEC, ChefApply::Dist::DK, ChefApply::Dist::WORKSTATION, ChefApply::Dist::DOWNLOADS_URL))

UI::Terminal.output(T.error.unsupported_installation(ChefApply::Dist::RUNEXEC, ChefApply::Dist::DK, ChefApply::Dist::WORKSTATION, ChefApply::Dist::DOWNLOADS_URL, ChefApply::Dist:: WORKSTATION_URL_SUFFIX))

bobchaos requested review from a team as code owners November 12, 2019 18:37

tyler-ball approved these changes Nov 12, 2019

View reviewed changes

afiune reviewed Nov 13, 2019

View reviewed changes

afiune approved these changes Jan 28, 2020

View reviewed changes

bobchaos force-pushed the dist branch from 5ff4c4a to 8474834 Compare January 28, 2020 22:08

afiune added Aspect: Packaging Distribution of the projects 'compiled' artifacts. Status: Adopted An issue that is being worked on. Triage: Confirmed Indicates and issue has been confirmed as described. labels Feb 12, 2020

afiune assigned marcparadise Feb 12, 2020

ramereth force-pushed the dist branch from 8474834 to fc48e5c Compare June 7, 2020 07:00

marcparadise requested changes Jun 10, 2020

View reviewed changes

bobchaos and others added 2 commits June 10, 2020 08:58

implement distro constants

7b17f03

Signed-off-by: Marc Chamberland <chamberland.marc@gmail.com>

chefstyle fixes

2b2e16a

Signed-off-by: Lance Albertson <lance@osuosl.org>

ramereth force-pushed the dist branch from fc48e5c to 2b2e16a Compare June 10, 2020 15:59

tyler-ball suggested changes Jul 8, 2020

View reviewed changes

	description: Show the current version of Chef Run.
	description: Show the current version of %1.

	UI::Terminal.output(T.error.unsupported_installation(ChefApply::Dist::RUNEXEC, ChefApply::Dist::DK, ChefApply::Dist::WORKSTATION, ChefApply::Dist::DOWNLOADS_URL))
	UI::Terminal.output(T.error.unsupported_installation(ChefApply::Dist::RUNEXEC, ChefApply::Dist::DK, ChefApply::Dist::WORKSTATION, ChefApply::Dist::DOWNLOADS_URL, ChefApply::Dist:: WORKSTATION_URL_SUFFIX))

Conversation

bobchaos commented Nov 12, 2019

Description

Related Issue

Types of changes

Checklist:

Uh oh!

tyler-ball left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobchaos Feb 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobchaos Nov 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobchaos commented Jan 28, 2020

Uh oh!

bobchaos commented Apr 26, 2020

Uh oh!

ramereth commented Jun 9, 2020

Uh oh!

marcparadise left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bobchaos Feb 6, 2020 •

edited

Loading

bobchaos Nov 13, 2019 •

edited

Loading

marcparadise left a comment •

edited

Loading