Skip to content

Conversation

iankronquist
Copy link
Contributor

@iankronquist iankronquist commented Nov 13, 2019

Octal numbers can't have 8 in them ;)

changelog: none

Octal numbers can't have 8 in them ;)
/// - Recommends a signed suffix, even though the number might be too big and an unsigned
/// suffix is required
/// - Does not match on `_128` since that is a valid grouping for decimal and octal numbers
/// - Does not match on `_128` since that is a valid grouping for decimal and hexadecimal numbers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// - Does not match on `_128` since that is a valid grouping for decimal and hexadecimal numbers
/// - Does not match on `_127` since that is a valid grouping for decimal and octal numbers

It's not a valid grouping for hex number though 😉

@JohnTitor JohnTitor added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Nov 14, 2019
@flip1995
Copy link
Member

ping from triage @iankronquist. Can you apply the suggestion, so we can merge this PR?

@iankronquist
Copy link
Contributor Author

@flip1995 sorry about the delay. Hope that fixes it.

@flip1995
Copy link
Member

I'd rather change the example to a valid octal number and keep the "decimal and octal" part.

@flip1995
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Feb 15, 2020

📌 Commit aef2662 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Feb 15, 2020

⌛ Testing commit aef2662 with merge 779b6ae...

bors added a commit that referenced this pull request Feb 15, 2020
Typo in literal_representation.rs

Octal numbers can't have 8 in them ;)

changelog: none
@bors
Copy link
Contributor

bors commented Feb 15, 2020

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing 779b6ae to master...

@bors bors merged commit aef2662 into rust-lang:master Feb 15, 2020
bors added a commit that referenced this pull request Feb 19, 2020
…logiq,flip1995

I like to move it, move it

GHA now runs in the background for 6 days (#5088)

Since then ~~15~~ 19 PRs were successfully merged and Travis+Appveyor agreed on the status in every case. ([GitHub PR search query](https://github.com/rust-lang/rust-clippy/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Amerged+merged%3A%3E%3D2020-02-12T15%3A42%3A00+sort%3Aupdated-desc+NOT+%5Bgh-pages%5D+in%3Atitle))

Some PRs were:
- #5163
- #5170
- #5168
- #5173
- #5171
- #5156
- #4809
- #5177
- #5182
- #5183
- #5184
- #5185
- #5186
- #5181
- #5189

Bug with GHA:
- When a rustc PR gets merged between the `integration_build` and the `integration` job, the `integration` job will fail. This happened once in #5162, but not in the past 6 days. Even if it would happen every 4th PR we would save time, since splitting up the integration build and tests saves 5-7 minutes per run and a complete run takes 15-17 minutes
- Sometimes the MacOS build takes up to an hour to download the master toolchain. Until now, this happend 2 or 3 times and can be resolved by a `@bors r3try`+canceling the previous run (restarting single jobs is not supported yet)

## Before merging this, rust-lang/rust-central-station#578 has to get merged

This PR is for starting the discussion and to get consensus (@rust-lang/clippy) on a final move to GHA. If we're ready, I'll contact Pietro, to finalize the move.

changelog: Clippy completely runs on GHA now 🎉

---

BTW: The deployment already runs on GHA, instead of Travis.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants