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

Emit annotations #11

Open
mgeisler opened this issue Apr 4, 2023 · 0 comments
Open

Emit annotations #11

mgeisler opened this issue Apr 4, 2023 · 0 comments

Comments

@mgeisler
Copy link

mgeisler commented Apr 4, 2023

GitHub actions can emit annotations which show up as notices/warnings/errors on the pull request. See the GitHub documentation.

It would be cool if dprint/check could emit them. I just checked the implementation of the action here and I see that it delegates directly to dprint check. Please let me know if you would prefer me to recreate this issue in the dprint repository.

mgeisler added a commit to google/comprehensive-rust that referenced this issue Sep 1, 2023
This is the result of two temporary changes:

- removing `src/` from the list of excluded directories,
- removing the `rustfmt` configuration.

The effect of the first change is that we normalize the formatting of
all Markdown files. The effect of the second is that we avoid
reformatting code snippets in the Markdown files. I want to reformat
them in a follow-up step since some of the changes seem unnecessary or
undesirable.

Thanks to mdbook-i18n-helpers 0.2, the POT file is nearly unchanged
after this, meaning that all existing translations remain valid. A few
messages were changed because of stray whitespace characters:

     msgid ""
     "Slices always borrow from another object. In this example, `a` has to remain "
    -"'alive' (in scope) for at least as long as our slice. "
    +"'alive' (in scope) for at least as long as our slice."
     msgstr ""

I have not updated CI to require perfectly formatted files. I think
doing so might be annoying since it makes it much harder to submit
small fixes directly from the GitHub editor. So for now, I suggest
that we simply run `dprint` by hand every few weeks. If

  dprint/check#11

is fixed so that `dprint` can annotate the lines that need fixing
directly, then I think we can consider more strict formatting checks.
mgeisler added a commit to google/comprehensive-rust that referenced this issue Sep 1, 2023
This is the result of two temporary changes:

- removing `src/` from the list of excluded directories,
- removing the `rustfmt` configuration.

The effect of the first change is that we normalize the formatting of
all Markdown files. The effect of the second is that we avoid
reformatting code snippets in the Markdown files. I want to reformat
them in a follow-up step since some of the changes seem unnecessary or
undesirable.

Thanks to mdbook-i18n-helpers 0.2, the POT file is nearly unchanged
after this, meaning that all existing translations remain valid. A few
messages were changed because of stray whitespace characters:

     msgid ""
     "Slices always borrow from another object. In this example, `a` has to remain "
    -"'alive' (in scope) for at least as long as our slice. "
    +"'alive' (in scope) for at least as long as our slice."
     msgstr ""

I have not updated CI to require perfectly formatted files. I think
doing so might be annoying since it makes it much harder to submit
small fixes directly from the GitHub editor. So for now, I suggest
that we simply run `dprint` by hand every few weeks. If

  dprint/check#11

is fixed so that `dprint` can annotate the lines that need fixing
directly, then I think we can consider more strict formatting checks.
mgeisler added a commit to google/comprehensive-rust that referenced this issue Sep 1, 2023
This is the result of running `dprint fmt` after making two temporary
changes:

- removing `src/` from the list of excluded directories,
- removing the `rustfmt` configuration.

The effect of the first change is that we normalize the formatting of
all Markdown files. The effect of the second is that we avoid
reformatting code blocks in the Markdown files. I want to reformat
them in a follow-up step since some of the changes seem unnecessary or
undesirable.

Thanks to mdbook-i18n-helpers 0.2, the POT file is nearly unchanged
after this, meaning that all existing translations remain valid. A few
messages were changed because of stray whitespace characters:

     msgid ""
     "Slices always borrow from another object. In this example, `a` has to remain "
    -"'alive' (in scope) for at least as long as our slice. "
    +"'alive' (in scope) for at least as long as our slice."
     msgstr ""

I have not updated CI to require perfectly formatted files. I think
doing so might be annoying since it makes it much harder to submit
small fixes directly from the GitHub editor. So for now, I suggest
that we simply run `dprint` by hand every few weeks. If

  dprint/check#11

is fixed so that `dprint` can annotate the lines that need fixing
directly, then I think we can consider more strict formatting checks.
mgeisler added a commit to google/comprehensive-rust that referenced this issue Dec 8, 2023
This is the result of running `dprint fmt` after making two temporary
changes:

- removing `src/` from the list of excluded directories,
- removing the `rustfmt` configuration.

The effect of the first change is that we normalize the formatting of
all Markdown files. The effect of the second is that we avoid
reformatting code blocks in the Markdown files. I want to reformat the
Rust code in a follow-up step since some of the changes seem
unnecessary or undesirable.

Thanks to mdbook-i18n-helpers 0.2, the POT file is nearly unchanged
after this, meaning that all existing translations remain valid. A few
messages were changed because of stray whitespace characters:

     msgid ""
     "Slices always borrow from another object. In this example, `a` has to remain "
    -"'alive' (in scope) for at least as long as our slice. "
    +"'alive' (in scope) for at least as long as our slice."
     msgstr ""

I have not updated CI to require perfectly formatted files. I think
doing so might be annoying since it makes it much harder to submit
small fixes directly from the GitHub editor. So for now, I suggest
that we simply run `dprint` by hand every few weeks.

If dprint/check#11 is fixed so that `dprint` can annotate the lines
that need fixing directly, then I think we can consider more strict
formatting checks.
mgeisler added a commit to google/comprehensive-rust that referenced this issue Dec 30, 2023
This is the result of running `dprint fmt` after removing `src/` from
the list of excluded directories.

This also reformats the Rust code: we might want to tweak this a bit
in the future since some of the changes removes the hand-formatting.
Of course, this formatting can be seen as a mis-feature, so maybe this
is good overall.

Thanks to mdbook-i18n-helpers 0.2, the POT file is nearly unchanged
after this, meaning that all existing translations remain valid. A few
messages were changed because of stray whitespace characters:

     msgid ""
     "Slices always borrow from another object. In this example, `a` has to remain "
    -"'alive' (in scope) for at least as long as our slice. "
    +"'alive' (in scope) for at least as long as our slice."
     msgstr ""

The formatting is enforced in CI and we will have to see how annoying
this is in practice for the many contributors. If it becomes annoying,
we should look into fixing dprint/check#11 so that `dprint` can
annotate the lines that need fixing directly, then I think we can
consider more strict formatting checks.
mgeisler added a commit to google/comprehensive-rust that referenced this issue Dec 30, 2023
This is the result of running `dprint fmt` after removing `src/` from
the list of excluded directories.

This also reformats the Rust code: we might want to tweak this a bit
in the future since some of the changes removes the hand-formatting.
Of course, this formatting can be seen as a mis-feature, so maybe this
is good overall.

Thanks to mdbook-i18n-helpers 0.2, the POT file is nearly unchanged
after this, meaning that all existing translations remain valid. A few
messages were changed because of stray whitespace characters:

     msgid ""
     "Slices always borrow from another object. In this example, `a` has to remain "
    -"'alive' (in scope) for at least as long as our slice. "
    +"'alive' (in scope) for at least as long as our slice."
     msgstr ""

The formatting is enforced in CI and we will have to see how annoying
this is in practice for the many contributors. If it becomes annoying,
we should look into fixing dprint/check#11 so that `dprint` can
annotate the lines that need fixing directly, then I think we can
consider more strict formatting checks.
mgeisler added a commit to google/comprehensive-rust that referenced this issue Dec 30, 2023
This is the result of running `dprint fmt` after removing `src/` from
the list of excluded directories.

This also reformats the Rust code: we might want to tweak this a bit
in the future since some of the changes removes the hand-formatting.
Of course, this formatting can be seen as a mis-feature, so maybe this
is good overall.

Thanks to mdbook-i18n-helpers 0.2, the POT file is nearly unchanged
after this, meaning that all existing translations remain valid. A few
messages were changed because of stray whitespace characters:

     msgid ""
     "Slices always borrow from another object. In this example, `a` has to remain "
    -"'alive' (in scope) for at least as long as our slice. "
    +"'alive' (in scope) for at least as long as our slice."
     msgstr ""

The formatting is enforced in CI and we will have to see how annoying
this is in practice for the many contributors. If it becomes annoying,
we should look into fixing dprint/check#11 so that `dprint` can
annotate the lines that need fixing directly, then I think we can
consider more strict formatting checks.
mgeisler added a commit to google/comprehensive-rust that referenced this issue Dec 30, 2023
This is the result of running `dprint fmt` after removing `src/` from
the list of excluded directories.

This also reformats the Rust code: we might want to tweak this a bit
in the future since some of the changes removes the hand-formatting.
Of course, this formatting can be seen as a mis-feature, so maybe this
is good overall.

Thanks to mdbook-i18n-helpers 0.2, the POT file is nearly unchanged
after this, meaning that all existing translations remain valid. A few
messages were changed because of stray whitespace characters:

     msgid ""
     "Slices always borrow from another object. In this example, `a` has to remain "
    -"'alive' (in scope) for at least as long as our slice. "
    +"'alive' (in scope) for at least as long as our slice."
     msgstr ""

The formatting is enforced in CI and we will have to see how annoying
this is in practice for the many contributors. If it becomes annoying,
we should look into fixing dprint/check#11 so that `dprint` can
annotate the lines that need fixing directly, then I think we can
consider more strict formatting checks.
mgeisler added a commit to google/comprehensive-rust that referenced this issue Dec 30, 2023
This is the result of running `dprint fmt` after removing `src/` from
the list of excluded directories.

This also reformats the Rust code: we might want to tweak this a bit in
the future since some of the changes removes the hand-formatting. Of
course, this formatting can be seen as a mis-feature, so maybe this is
good overall.

Thanks to mdbook-i18n-helpers 0.2, the POT file is nearly unchanged
after this, meaning that all existing translations remain valid! A few
messages were changed because of stray whitespace characters:

     msgid ""
     "Slices always borrow from another object. In this example, `a` has to remain "
    -"'alive' (in scope) for at least as long as our slice. "
    +"'alive' (in scope) for at least as long as our slice."
     msgstr ""

The formatting is enforced in CI and we will have to see how annoying
this is in practice for the many contributors. If it becomes annoying,
we should look into fixing dprint/check#11 so that `dprint` can annotate
the lines that need fixing directly, then I think we can consider more
strict formatting checks.

I added more customization to `rustfmt.toml`. This is to better emulate
the dense style used in the course:

- `max_width = 85` allows lines to take up the full width available in
our code blocks (when taking margins and the line numbers into account).
- `wrap_comments = true` ensures that we don't show very long comments
in the code examples. I edited some comments to shorten them and avoid
unnecessary line breaks — please trim other unnecessarily long comments
when you see them! Remember we're writing code for slides 😄
- `use_small_heuristics = "Max"` allows for things like struct literals
and if-statements to take up the full line width configured above.

The formatting settings apply to all our Rust code right now — I think
we could improve this with dprint/dprint#711
which lets us add per-directory `dprint` configuration files. However,
the `inherit: true` setting is not yet implemented (as far as I can
tell), so a nested configuration file will have to copy most or all of
the top-level file.
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

No branches or pull requests

1 participant