Skip to content

remove unneeded indent #937

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

Closed
wants to merge 1 commit into from

Conversation

Mashimiao
Copy link

Signed-off-by: Ma Shimiao [email protected]

Some indents will cause erorr, we should remove them.
Some places are similar with #936

@@ -46,6 +46,6 @@ Specifications have a variety of different timelines in their lifecycle.
This means a major release like a v1.0.0 or v2.0.0 release will take 1 month at minimum: one week for rc1, one week for rc2, one week for rc3, and one week for the major release itself.
Maintainers SHOULD strive to make zero breaking changes during this cycle of release candidates and SHOULD restart the three-candidate count when a breaking change is introduced.
For example if a breaking change is introduced in v1.0.0-rc2 then the series would end with v1.0.0-rc4 and v1.0.0.
- Minor and patch releases SHOULD be made on an as-needed basis.
* Minor and patch releases SHOULD be made on an as-needed basis.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice. I've submitted this upstream with opencontainers/project-template#40.

config-linux.md Outdated
* **`architectures`** *(array of strings, OPTIONAL)* - the architecture used for system calls.
A valid list of constants as of libseccomp v2.3.2 is shown below.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you removing this line? A blank line before and after a Markdown list is traditional, and it renders fine.

Copy link
Author

Choose a reason for hiding this comment

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

I removed them, because the blank line does not make sense.
But if it's traditional, I will keep them.

config-linux.md Outdated
* **`names`** *(array of strings, REQUIRED)* - the names of the syscalls.
`names` MUST contain at least one entry.
* **`action`** *(string, REQUIRED)* - the action for seccomp rules.
A valid list of constants as of libseccomp v2.3.2 is shown below.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want to keep this pre-list (and the later post-list) blank lines too. There are a number of other instances of this as well.

glossary.md Outdated
@@ -34,5 +34,4 @@ On Linux, the namespaces from which new [container namespaces](#container-namesp

[JSON]: https://tools.ietf.org/html/rfc7159
[UTF-8]: http://www.unicode.org/versions/Unicode8.0.0/ch03.pdf

Copy link
Contributor

Choose a reason for hiding this comment

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

From the style docs:

Referenced links should be kept in two alphabetically sorted sets, a general reference section followed by a man page section.

I'm fine dropping that and going to a single alphabetically sorted list, but if we want to do that we should also update the style docs. I'm also fine keeping the separation and the blank line here.

Copy link
Author

Choose a reason for hiding this comment

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

If styles requries, by now I have no intention to change it. Let's keep where it was.

@Mashimiao
Copy link
Author

updated based on comments, PTAL

@TomSweeneyRedHat
Copy link

LGTM, nice touch ups.

config-linux.md Outdated

A valid list of constants as of libseccomp v2.3.2 is shown below.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you want to keep this line where it was (as part of the initial paragraph, with a blank line after it separating the sub-list).

Copy link
Author

Choose a reason for hiding this comment

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

updated.

While this property is OPTIONAL, some values of `defaultAction` are not useful without `syscalls` entries.
For example, if `defaultAction` is `SCMP_ACT_KILL` and `syscalls` is empty or unset, the kernel will kill the container process on its first syscall.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this “you'll probably want to set this” text is worth keeping in its own paragraph. But I don't feel too strongly about it, and am ok with all of this getting squashed into a single paragraph like you're doing here.

Signed-off-by: Ma Shimiao <[email protected]>
@vbatts
Copy link
Member

vbatts commented Apr 4, 2018

rebase please. This LGTM.

@vbatts
Copy link
Member

vbatts commented Dec 17, 2019

house keeping: if someone else wants to pick up these commits and rebase, feel free to. It's been 1.5yrs needing only a rebase.

@h-vetinari h-vetinari mentioned this pull request Feb 3, 2020
Comment on lines -469 to +470
If `l3CacheSchema` is set, runtimes MUST write the value to the `schemata` file in the `<container-id>` directory discussed in `intelRdt`.

If `l3CacheSchema` is not set, runtimes MUST NOT write to `schemata` files in any `resctrl` pseudo-filesystems.
* If `l3CacheSchema` is set, runtimes MUST write the value to the `schemata` file in the `<container-id>` directory discussed in `intelRdt`.
* If `l3CacheSchema` is not set, runtimes MUST NOT write to `schemata` files in any `resctrl` pseudo-filesystems.
Copy link
Contributor

@h-vetinari h-vetinari Feb 5, 2020

Choose a reason for hiding this comment

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

Tagging this for reference in rebased PR #1031

@h-vetinari h-vetinari mentioned this pull request Feb 5, 2020
tianon added a commit that referenced this pull request Feb 5, 2020
@tianon
Copy link
Member

tianon commented Feb 5, 2020

Rebased/merged in #1031 👍

Thanks!

@tianon tianon closed this Feb 5, 2020
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.

6 participants