Fix minor formatting issues

[stefansundin]

Issue number:

N/A

Description of changes:

I have been experimenting with Bottlerocket recently and I've been a bit annoyed that when saving some files, my editor often fixes up some whitespaces unrelated to my change, like whitespace at the end of a line or missing newline at the end of files.

I also noticed that some documentation was out of date, and I had .DS_Store files showing up in my working tree. So I decided to take a pass to find and fix as many of these small problems as possible in one go and try to get it merged upstream.

Please let me know if there's any change you don't agree with and I can revert those changes. Thanks!

Testing done:

I've rendered some of the documentation pages and verified that they still look good. I found some code formatting were using the wrong tag which should now be fixed.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

[stefansundin]

I have tried to build an image using a 1 MB EFI partition but that failed. Using a 2 MB partition worked fine.

[stmcginnis]

Awesome, a lot of great clean up here. This would get us pretty close to being able to add the mdlint job that we use in some of the other repos.

If you do an update, please address the .gitignore comment. Otherwise I don't think that's too critical, so while I'd prefer not to have it, I don't think it's a big deal if we include it.

[etungsten]

Thanks for the fixes

[etungsten]

I'm wondering what this change does? It seems like it makes no difference in the rich diff. The reason I ask is because all the READMEs that have this are auto-generated from docstrings from our source code, so I worry that this change won't last when we touch the source in any way.

[jpmcb]

I'm wondering the same thing: I'm not seeing a change in the GitHub UI and it doesn't appear to be adding a newline at the end?

[stefansundin]

Yes, it is the newline at EOF. It was missing in many of these files. It is strange the GitHub is not rendering the symbol that it renders in some views.

It's automatically added when I run cargo readme > README.md. I see that there's a generate-readme package that is supposed to automatically generate these but I don't know if there's an easy way to invoke this. I suppose it happens at build time. I just built this and there were no changes to the README files.

Here's an alternative way to view the diff: https://github.com/bottlerocket-os/bottlerocket/commit/d5487d348f6bbf1e8c2ea973d0f23441d37d88fc.diff

diff --git a/sources/api/certdog/README.md b/sources/api/certdog/README.md
index 8769328a9c..e87b8418e0 100644
--- a/sources/api/certdog/README.md
+++ b/sources/api/certdog/README.md
@@ -8,4 +8,4 @@ Current version: 0.1.0
 
 ## Colophon
 
-This text was generated using [cargo-readme](https://crates.io/crates/cargo-readme), and includes the rustdoc from `src/main.rs`.
\ No newline at end of file
+This text was generated using [cargo-readme](https://crates.io/crates/cargo-readme), and includes the rustdoc from `src/main.rs`.

[jpmcb]

Awesome!! 👏🏼 Thanks for the hard work on this one - huge value add!

[bcressey]

Nice, thank you for doing this!

[bcressey]

Most of these edits end up being not stable across a run where README.md is recreated - if I check out your branch and do something like this, the newlines are lost:

cd sources
find -name README.md -delete
export VARIANT=aws-dev
cargo build

To make them permanent, you'd need to change the generate_readme helper to write the trailing newline, since this is what each crate's build script invokes. The offending code is here:

bottlerocket/sources/generate-readme/src/lib.rs

Line 87 in b530f30

let mut readme = File::create("README.md").context(error::ReadmeCreateSnafu)?;

[stefansundin]

This seems to be a problem in cargo-readme where running cargo readme > README.md can produce a different result than when using cargo_readme::generate_readme() 😞

There's a call to template.trim_end_matches("\n") here: https://github.com/livioribeiro/cargo-readme/blob/11ce95834c5ef52922174a22cd91e9d1868cf3d4/src/readme/template.rs#L54

I suspect that when cargo readme is called a newline is added at the end at some other part of the code.

Anyway, I made a change to generate_readme to append a newline. Hopefully that is good enough.

It might make sense to try to fix this in cargo-readme, but it looks like there's not much development there anymore. Let me know if you want me to revert these newlines.

[webern]

Than you for this PR.

I was writing notes on how we generate our readmes before I saw your conversation with @bcressey and your change to the generate_readme function. Thank you for figuring that out.

If you haven't recently, can go into sources and export VARIANT=aws-dev && cargo build and see that there is no git diff after building?

[webern]

This would need to be changed in the README.tpl file if it's a change we care about. I can't tell from this GitHub diff rendering what was changed.

[stefansundin]

Thanks for the review @webern.

If you haven't recently, can go into sources and export VARIANT=aws-dev && cargo build and see that there is no git diff after building?

I tried this but I got an error from one package that I'm not running Linux. I assume that when I run the actual build inside of Docker it won't propagate these kinds of things to my git worktree, right?

I can run this on a Linux machine later today.

[webern]

I tried this but I got an error from one package that I'm not running Linux. I assume that when I run the actual build inside of Docker it won't propagate these kinds of things to my git worktree, right?

Yeah, there are Linux things in there and I found it very hard to cross compile from a mac when I tried. This seems to work though. First I perturbed a generated readme file and committed that change. Then I compiled with this (from the root of the Bottlerocket git tree):

docker run \
    --mount "type=bind,src=$(pwd)/sources,dst=/sources" \
    --workdir="/sources" \
    --name build-bottlerocket-sources \
    --rm \
    -e VARIANT=aws-k8s-1.23 \
    --user root \
    public.ecr.aws/bottlerocket/bottlerocket-sdk-x86_64:v0.28.0 \
    cargo build

This did show that the README was out-of-sync

$ git status
On branch delete-this-branch
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   sources/models/README.md

[stefansundin]

Thanks @webern. I just ran that and no files changed. 🎉

[zmrow]

🌋

Thanks!

[jpmcb]

👏🏼

[stmcginnis]

Shoot, there's a merge conflict now. Would you mind rebasing on the latest from develop? Then we can try to wrap this up quickly so we don't run into any more conflicts.

[stefansundin]

@stmcginnis Done!

[jpmcb]

Rebase looks good!

[stmcginnis]

Let's get this in! Would be good to have feedback from a few of the folks that had commented earlier, but since this is a holiday week in the US it's probably not worth waiting in the hopes of getting that. Easy enough to follow up with any new PRs if there is anything in these changes that are of concern.