Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Mixer documentation #80

Merged
merged 11 commits into from
Mar 7, 2018

Conversation

kevincwells
Copy link
Contributor

Updating the documentation for mixer to account for recent changes,
and to better clarify some concepts. This is to coincide with an
upcoming release of mixer, in which several important changes will
be introduced.

  1. The mixer CLI has been rewritten, supporting POSIX-style flags
    and a modern command layout. This is a breaking change, as commands
    and flags have been renamed.

  2. The initialization steps for mixer have been further automated,
    so the documentation has been updated to account for the new,
    simpler approach.

  3. Documentation has been added for the recent command for
    automating the addition of bundles to a mix.

  4. The tool now handles caching upstream CLR bundle definitions
    in a more automated way, and so the 'mixer get-bundles' command
    (and its corresponding documentation section) has been removed.

Signed-off-by: Kevin C. Wells kevin.c.wells@intel.com

@kevincwells
Copy link
Contributor Author

@tmarcu -- please read this over and verify its accuracy. The diff might not be incredibly useful (effectively the entire file has been changed at least slightly), so you might want to just read the final file.

This PR has a lot of little clean-up throughout, trying to consistently use the :file:`path/to/file.ext` and other inline formatting techniques. That said, I notice clearlinux.org doesn't actually take advantage of many of these, as the stylesheets render much of these modified texts the same.

This builds and appears to render correctly for me, but I am not pulling in all the final stylesheets used on clearlinux.org.

Updating the documentation for mixer to account for recent changes,
and to better clarify some concepts. This is to coincide with an
upcoming release of mixer, in which several important changes will
be introduced.

1) The mixer CLI has been rewritten, supporting POSIX-style flags
and a modern command layout. This is a breaking change, as commands
and flags have been renamed.

2) The initialization steps for mixer have been further automated,
so the documentation has been updated to account for the new,
simpler approach.

3) Documentation has been added for the recent command for
automating the addition of bundles to a mix.

4) The tool now handles caching upstream CLR bundle definitions
in a more automated way, and so the 'mixer get-bundles' command
(and its corresponding documentation section) has been removed.

Signed-off-by: Kevin C. Wells <kevin.c.wells@intel.com>
@rcaballeromx
Copy link
Contributor

@kevincwells Thank your for this update. I'll review and provide comments. Regarding the inline markup: Not all classes have been implemented on the website's CSS but they are imperative for our automated document based testing efforts. Thank you for using them.

Merge branch 'mixer-doc-update' of https://github.com/kevincwells/clear-linux-documentation into mixer-doc-update
Signed-off-by: Ecouzens <evan.couzens@intel.com>

Behind the scenes, mixer uses a local cache of the upstream Clear Linux
bundle definitions. These are stored in the
:file:`.mixer/upstream-bundles/clr-bundles-{VER}/bundles/` directory in your
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevincwells I'm sure this doesn't need to be said, but this kind of content will have to be updated as the new bundle/folder PR changes go in soon :)

tmarcu
tmarcu previously approved these changes Feb 7, 2018
ECouzens
ECouzens previously approved these changes Feb 13, 2018
Copy link
Contributor

@ECouzens ECouzens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and committed changes

@matthewrsj
Copy link
Contributor

bump

@tmarcu
Copy link
Contributor

tmarcu commented Feb 14, 2018

@rcaballeromx @ECouzens What is going on with this PR...?

@ECouzens
Copy link
Contributor

@tmarcu apologies . We're waiting for an additional SW release this week before we merge this PR. @bktan8 will verify when that's released and will comment here when we are go for merge

@tmarcu
Copy link
Contributor

tmarcu commented Feb 22, 2018

😀

@kevincwells
Copy link
Contributor Author

@rcaballeromx @ECouzens -- There have been new additions to the mixer tool since this PR was added. I'd like to update the docs to reflect them, but I'm a bit stuck with this PR still outstanding.

Would you prefer I:

  1. Update this branch (force push) with new content
  2. Add a new commit to this branch with new content
  3. Start a new branch (and submit a new PR) that is, temporarily, based on top of the commits in this branch, and then rebase on top of master once this is merged?

I don't want to mess up your validation/merge process, so I won't make any changes to this branch without your approval.

@mrkz
Copy link
Contributor

mrkz commented Feb 27, 2018

@rcaballeromx ^

@rcaballeromx
Copy link
Contributor

@kevincwells The correct answer is option 2. Just as you would in the code, please make any updates on this content on this branch but in different commits. Thanks!

@tmarcu
Copy link
Contributor

tmarcu commented Feb 27, 2018

The new Mixer is now released in upstream, so this documentation has to get in ASAP. @kevincwells is this already ready to push on your end?

@kevincwells
Copy link
Contributor Author

kevincwells commented Feb 27, 2018

@rcaballeromx -- I'm definitely willing to do that, but wouldn't that effectively invalidate the two approvals and other review that has gone through on this PR? The new commits would need to go through a new approval.

Is there actually still something preventing this commit from being merged?

@kevincwells
Copy link
Contributor Author

@tmarcu -- It's about half done and is my top priority today.

@rcaballeromx
Copy link
Contributor

@kevincwells You are absolutely correct, just as it would with code. I haven't merged this content because based on the comments, it is clear that it is not complete.
As soon as you complete the content, I will review. This is also my top priority for today.

@@ -70,211 +92,267 @@ Mixing
VERSIONURL=<URL where the version of the mix will be hosted>
FORMAT=1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This builder.conf is missing the [Server] and [Mixer] sections:

[Mixer]
LOCAL_BUNDLE_DIR=/home/clr/mix/local-bundles

[Server]
debuginfo_banned=true
debuginfo_lib=/usr/lib/debug/
debuginfo_src=/usr/src/debug/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bktan8 Thanks for taking a look at this. Yes, this is one of the changes coming in the update for 4.0


This command copies the specified bundle defintion files from your
configured upstream version of Clear Linux (:file:`.clearversion`) into
your :file:`mix-bundles` directory.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the "mix-bundles" directory in my testing. I see a "mixbundles" file which contains a list of bundles.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, yes, while these docs have been valid for the last month, new changes have been made in that time that make much of this outdated. I'm updating the documentation as we speak.

@tmarcu
Copy link
Contributor

tmarcu commented Feb 27, 2018

@bktan8 @rcaballeromx please do not review this PR any more until the new commits are in.

This was complete with multiple approvals, but it got mangled with absolutely irrelevant commits and then left to get stale for a month.

@bktan8
Copy link
Contributor

bktan8 commented Feb 27, 2018

got it!

@rcaballeromx
Copy link
Contributor

@tmarcu Understood. I won't review until you or @kevincwells let me know.

@kevincwells
Copy link
Contributor Author

@rcaballeromx @ECouzens @bktan8 -- I've pushed a new commit with the updates to cover what's new in Mixer 4.0.

@ECouzens -- I liked what you were trying to do with the jump-to-label tag for "Step 4", but I had to remove it, because it broke the auto-numbering for the list. It made that section start a new count, so that section was no longer being numbered "4.", but instead "1."

@tmarcu tmarcu dismissed stale reviews from ECouzens and themself February 28, 2018 19:54

new changes

@kevincwells
Copy link
Contributor Author

@rcaballeromx -- @bktan8 has pointed out a couple of places that need updated. I can submit a new push in the next half hour that fixes these things. I'll post here when I do, so please be sure to pull the latest version.

RPM files and not corrupt.

#. **Add/remove/edit/list the bundles in your mix**. The bundles in your mix are
specified in the Mix Bundle List. This list is stored as a flat file called
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevincwells - is there a reason M, B, and L are capitalized in "Mix Bundle List"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to get into a grammar argument about what constitutes a "proper noun", but the short version is that I was treating it as one, a) to improve clarity and help people understand that this is a specific, unique thing, and b) to be consistent with the way it is written throughout all of the new command help doc information.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do both a without the capitalization. Sadly, b means that the help doc must be revised at a later date. As any writer can attest, you cannot fight grammar. I'll make sure to mark it clearly to achieve the desired clarity and specificity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there actually a reason to remove these capitalizations, here and throughout the commands and code comments? Just to be completely clear, the capitalization is not grammatically incorrect. It might be against our adopted style guide (do we actually have one?), but that's a different concept altogether.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevincwells Yes there is. The dictionary is clear: https://en.oxforddictionaries.com/spelling/using-capital-letters
We have had a style guide for months. You can find the capitalization rules here: https://clearlinux.org/documentation/clear-linux/reference/collaboration/documentation/grammar
I will follow both the Oxford grammar and our style guide in my final tweaks.

Copy link
Contributor Author

@kevincwells kevincwells Mar 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rcaballeromx

Thanks for linking me to the style guide -- when I asked if we have one, that was indeed a serious question. I have bookmarked the page and will do my best to adhere to it.

In regards to the style guide and the OED link -- I see nothing at all at either link that disputes this capitalization. The OED link is specifically a list of situations in which "you should always use a capital letter" -- it makes no assertions about when you should not. To my surprise, it makes no mention of the word "proper noun", which is often one of the cases cited. The style guide, however, does state that. "Mix Bundle List" is, in fact, a proper noun.

OED's definition for proper noun is quite open, defining it through contrast to a common noun: "A noun denoting a class of objects or a concept as opposed to a particular individual."

Merriam-Webster, Dictionary.com, and Wikipedia make the distinction in both their definition of proper noun and common noun: a "proper noun" is a noun that refers to a unique entity, while a "common noun" refers to a non-unique instance from within a class of entities.

While there are many lists, there is only one Mix Bundle List.

This same definition says that we should, in fact, actually capitalize "Mixer" when we are referring to it as the entity: there are many mixers (things that mix other things), but there is one Mixer (the tool to build Clear Linux update content). And that would 100% be in keeping with the style guide. Where we should not capitalize "mixer" (and the style guide addresses this when referring to not changing the case of software components) is when it is a literal command, referring to the lowercase-m binary executable.

As such, it would be appropriate to write: "To begin using Mixer, run mixer init."

@rcaballeromx
Copy link
Contributor

@kevincwells @bktan8 My day is almost over and I do need to change some markup and minor punctuation. Make your changes and I will pull them first thing Monday morning to make mine and have them ready by the time you get in the office.

This patch updates the documentation to comply with all of the new changes
Mixer 4.0 introduces:

- Many changes to the bundle lifecycle. It introduces
the concept of "local" and "upstream" bundles, and several new commands
to work with them.

- The new, native implementations of swupd-server
and the bundle-chroot-builder tool, both still in experimental
mode behind optional flags.

- New default values for mixer init

Signed-off-by: Kevin C. Wells <kevin.c.wells@intel.com>
@kevincwells
Copy link
Contributor Author

@rcaballeromx @bktan8 I've added the clarification to mixer bundle remove, and added the new section for the mixer versions update command. I've also run additional spellchecking and caught a few more.

Let me know if you run into any issues with your final tweaks tomorrow.


If you are creating RPMs from scratch, you can use ``autospec``, ``mock``,
``rpmbuild``, etc. to build them. If they are not built on Clear,
make sure your configuration and toolchain builds them correctly for Clear,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does it mean to build the RPM correctly for Clear? The text never explains.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good question. I didn't change the wording here (it appears as new lines, but it's just been reformatted) because I don't know enough about Clear's RPM requirements to articulate it.

Could @matthewrsj @phmccarty or someone elaborate on what could go here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can link to https://github.com/clearlinux/common for this section. I don't want to explain any of it in here though, because it will make it too long and off topic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tmarcu I agree, the link is sufficient.

@rcaballeromx
Copy link
Contributor

@kevincwells I am frankly not that worried about the capitalization issues. I have been fixing them as I go while editing and you can take a look at the final draft. Content-wise however, the text has more serious issues like excessive use of passive voice and relative pronouns, incorrect cross-referencing, and inconsistent use of terminology. I am working on a draft fixing all these issues but it is taking a lot longer than expected due to the complexity of the content. I am working hard at it to have a draft uploaded by today EoD.

@kevincwells
Copy link
Contributor Author

@rcaballeromx 😀 I wholeheartedly believe you and appreciate you fixing those things.

ECouzens and others added 3 commits March 5, 2018 11:29
The rewrite includes:
Explicit commands for important options.
Use of lists to improve the clarity of the configuration variables content.
Removal of command prompts to enable DBT.
Consistent use of terminology.
Use of concrete examples to improve the clarity of the commands.
Improved content flow and navigation with linked workflow overviews.
Added cross-references.
Removed ambiguous relative pronouns and punctuation.
Removed passive voice whenever possible.
Changed verbs to present tense for consistency and style guide compliance.
Optimized headings for search engines.

Signed-off-by: Rodrigo Caballero <rodrigo.caballero.abraham@intel.com>
@rcaballeromx
Copy link
Contributor

@tpleavitt Thank you for the comments. @ECouzens is performing an additional copy edit to clean up all those typos and artifacts that get introduced by massive rewrites. @kevincwells @tmarcu Can you take a look and ensure the content is still technically accurate? I tried to keep the technical information intact while providing more explicit command examples. Lastly, keep in mind that all '#' command prompts were replaced with sudo to prepare the document for automated testing.

@kevincwells
Copy link
Contributor Author

@rcaballeromx I'm reviewing now. Very happy with the structural changes -- great work. I'm doing the technical comments as part of a formal "Review", so those will come in a few minutes when I'm done. 😄

Signed-off-by: Ecouzens <evan.couzens@intel.com>
Signed-off-by: Ecouzens <evan.couzens@intel.com>
Copy link
Contributor Author

@kevincwells kevincwells left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rcaballeromx -- Great rewrite! I love the new sections (way better than the fragile numbered list) and all of the additional example commands you inserted. You did a great job.

I've found a number of technical issues, though almost all are really minor. Almost all of the sudo's need to be dropped -- I noted each one individually because not all need to be dropped, and I wanted it to be explicitly clear which to remove. (Apologies in advance that this makes it look like there are a ton of edits; at least 40 of them are these one-word changes.) The rest of the technical issues are mostly details that got lost in translation.

I've marked all things that I feel need to be changed as "requested edits", and marked a few optional things as "suggested edits".

Because this is, technically, my own Pull Request, Github will not allow me to mark my review as "Request changes". I am definitely capable of making the changes I noted myself, but given that this commit is yours, I'd prefer if you edited it.

For reference, I find that the practice of your team pushing commits to my own personal repo's pull request branch leads to a lot of confusion on my part. I understand with a rewrite as extensive as this one, it makes sense for your team to do it yourselves and push a commit. In general, however, my personal branch should really only be edited by me.

#. `Create an image`_

Alternatively, the following workflow applies if the mix only uses |CL|
bundles.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is misleading. Technically, this workflow applies if the mix only uses Clear Linux content. The bundles themselves can be edited, or completely original, so long as they only include packages available in upstream CLR.

Suggested edit: just replace "bundles" with "content".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

specified in the builder.conf, in this case ``/home/clr/mix/mix-bundles``.
This is where the bundle definitions are stored for your mix, and it's where
the chroot-builder looks to know what bundles must be installed.
sudo mixer init
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sudo on these commands was stripped deliberately. Specifically, init should not be run with sudo, or the configuration content will be created and owned by root. It's not the end of the world if this happens (I don't think we need any kind of warning against doing so), we just shouldn't include the sudo in our instructions.

Requested edit: remove sudo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

based on the latest released upstream |CL| version. If a :file:`builder.conf`
file is not already present in your workspace, mixer creates a default configuration
file along with several version and tracking files and two bundle directories:
:file:`local-bundles` and :file:`upstream-bundles`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is misleading. This technically says that the version tracking and bundle directories are created if builder.conf is not already present. The creation of these files and directories is not tied to the existence of builder.conf.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split it into two sentences to reflect that the creations are independent from each other.


# mixer init --git
sudo mixer init --clear-version 21060 --mix-version 100
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested edit: remove sudo


.. code-block:: bash
Additionally, to build a mix with your own custom RPMs, use the optional
:option:`--local-rpms` flag, for example:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The leading space here makes this indent as if a tab was inserted. Is this intended?

Suggested edit: remove leading space.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not intended, nice catch! Done


.. code-block:: bash

sudo mixer versions update --upstream-version latest
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested edit: remove sudo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


This command sets the upstream version to the latest released version of
upstream |CL| within the same format version. The :command:`mixer
versions update` command does not allow to set an upstream version to a
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You removed the word "you", but without it, "The mixer versions update command does not allow to set" is incorrect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 'you' back

"format bump" build, which is currently a manual process. See :ref:`mixer-format`
for more information.

Learn which mix version or upstream version you currently are on with the
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to my comment about mixer bundle validate, the active voice (and thus command form) here kind of loses that this is an optional step. Not strictly wrong, it just feels weird.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 'Optionally, you can" to communicate the fact this is optional.


.. code-block:: bash

sudo mixer versions
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested edit: remove sudo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


Edit the config file to include all bundles that you want *preinstalled*
into your image. The rest of the bundles in your mix will be available to your users via:
sudo mixer bundle remove bundle1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested edit: remove sudo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@kevincwells
Copy link
Contributor Author

kevincwells commented Mar 6, 2018

@rcaballeromx Ah, I see @ECouzens just pushed a few new commits on top of yours that I was reviewing. This makes Github mark the vast majority of my edits as "outdated" (and hidden by default). Please do not miss these edits, as they are still valid.

@kevincwells
Copy link
Contributor Author

kevincwells commented Mar 6, 2018

@ECouzens -- I notice this branch now has a few merge commits (e.g. 888cf2f), in addition to the edit commits. I'm guessing you did a git merge into your own local copy of this branch, then pushed these commits along with your edits on accident. As is, this branch needs to be rebased to remove them. In the future, can you be sure to either pull the branch from scratch, do a git pull instead of a git merge, or manually rebase your changes before pushing them?

@kevincwells
Copy link
Contributor Author

@rcaballeromx and others:

I'd like to reiterate that the review process I've observed here is not how Git (or GitHub) are supposed to be used. When I submit a pull request, the commits live on my personal branch in my personal GitHub fork repository (in this case, kevincwells/mixer-tools). In general, no one should write to this branch except me. This is not a communal branch on a communal repository like the original, it is a personal fork of that original.

If there are changes you would like before my PR can be merged, and I can reasonably make them myself, you should note the changes as part of GitHub's "Request changes" review feature. It would then be my responsibility to make those changes on my personal branch, and then push (or force-push) to update my PR.

The fact that I can (and likely will need to) force-push changes to commits should illustrate why others should not be pushing to my branch. If I am making an edit to my commit, and some other person pushes additional commits to it before I finish, I can accidentally erase them by force-pushing my update. (Yes, this is a risk any time people are sharing a branch and editing it in parallel. But again, this is not, in fact, a shared branch.)

If your team would like to instead be responsible for editing the contents of my PR, perhaps the best process would be to have developers instead submit their PR's to a "review" branch on the upstream repository. Your team could then accept the PR and merge the commits into the review branch, where you would be free to make additional commits or edit those that already exist.

@rcaballeromx
Copy link
Contributor

@kevincwells Thank you very much for the feedback regarding this review process. In the past, we followed the process just as you described it and as Github was intended to work. However, that process creates an overhead. For example, if I review the content and comment on the changes providing the edits, you then need to implement said edits. Thus, both of us have performed the same work. I will take your feedback to our documentation team meeting and discuss alternatives and ways to avoid the issues.

@rcaballeromx
Copy link
Contributor

@kevincwells I will not miss your edits and thank you for making all the comments in such a precise way.

Additionally, all the unnecessary sudo commands have been removed.

Signed-off-by: Rodrigo Caballero <rodrigo.caballero.abraham@intel.com>
Copy link
Contributor

@rcaballeromx rcaballeromx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have addressed all your comments @kevincwells and marked them as done with further explanations where appropriate.

#. `Create an image`_

Alternatively, the following workflow applies if the mix only uses |CL|
bundles.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

specified in the builder.conf, in this case ``/home/clr/mix/mix-bundles``.
This is where the bundle definitions are stored for your mix, and it's where
the chroot-builder looks to know what bundles must be installed.
sudo mixer init
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

based on the latest released upstream |CL| version. If a :file:`builder.conf`
file is not already present in your workspace, mixer creates a default configuration
file along with several version and tracking files and two bundle directories:
:file:`local-bundles` and :file:`upstream-bundles`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split it into two sentences to reflect that the creations are independent from each other.


.. code-block:: bash
Additionally, to build a mix with your own custom RPMs, use the optional
:option:`--local-rpms` flag, for example:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not intended, nice catch! Done


#. **Configure builder.conf**. Edit the :file:`builder.conf` as needed.
sudo mixer init --local-rpms
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


.. code-block:: bash

sudo mixer versions update --upstream-version 21070
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


.. code-block:: bash

sudo mixer versions update --upstream-version latest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


This command sets the upstream version to the latest released version of
upstream |CL| within the same format version. The :command:`mixer
versions update` command does not allow to set an upstream version to a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 'you' back

"format bump" build, which is currently a manual process. See :ref:`mixer-format`
for more information.

Learn which mix version or upstream version you currently are on with the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 'Optionally, you can" to communicate the fact this is optional.


.. code-block:: bash

sudo mixer versions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@kevincwells
Copy link
Contributor Author

kevincwells commented Mar 7, 2018

+1 on all of @rcaballeromx's changes -- great job!

I found a missing word (my doing) and a very minor formatting issue, and just pushed a commit that fixes both.

The only outstanding things I am aware of are the two things I'd like @tmarcu and @matthewrsj to weigh in on:

  • Am I correct that mixer only auto-generates the Swupd_Root.pem file if the location defined in CERT doesn't exist? @rcaballeromx made that change, but I'm not an expert on that part of the code.
  • Should we, in fact, be instructing people to use the --new-swupd and --new-chroots yet? Or should we just say it is optional/experimental? (Either way, the docs will need updated once those are no longer behind the --new- flags)

@tmarcu
Copy link
Contributor

tmarcu commented Mar 7, 2018

@kevincwells
1.) Yes it will only generate if the file doesn't exist.
2.) yes we should be telling people to use the new flags.

Copy link
Contributor

@rcaballeromx rcaballeromx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevincwells @tmarcu Thank you both! I am approving and merging the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants