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

fix mdbook test with ```ignore #7832

Closed
wants to merge 1 commit into from
Closed

Conversation

guswynn
Copy link
Contributor

@guswynn guswynn commented Jan 25, 2020

The rust reference repo has mdbook test as part of its CI, so when I was working on a PR here, mdbook test being broken confused me, so I figured I would make a PR to fix it.

I added some ignores to the problematic blocks (basically those with dependencies or using env vars)
and fixed some links to use formatting instead of tab formatting

@ehuss ehuss self-assigned this Jan 25, 2020
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Each code block should use the appropriate language. text disables highlighting and better conveys the intent of what the code block is for.

@@ -30,7 +30,7 @@ library call as part of the build script.

First, let’s take a look at the directory structure of this package:

```
```ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```ignore
```text

@@ -128,7 +128,7 @@ a Rust library which calls into C to print “Hello, World!”.

Like above, let’s first take a look at the package layout:

```
```ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```ignore
```text

@@ -325,7 +325,7 @@ situations. For example, if you have a library where you want a *module* named
compile anything in the `bin` directory as an executable. Here is a sample
layout of this scenario:

```
```ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```ignore
```text

@@ -221,7 +221,9 @@ for permission if [crates.io] doesn’t have all the scopes it would like to.
An additional barrier to querying GitHub is that the organization may be
actively denying third party access. To check this, you can go to:

https://github.com/organizations/:org/settings/oauth_application_policy
```ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```ignore
```text

@@ -38,7 +38,7 @@ As with most config values, the index may be specified with an environment
variable instead of a config file. For example, setting the following
environment variable will accomplish the same thing as defining a config file:

```
```ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```ignore
```text

@@ -207,7 +207,7 @@ information about which commands would be run without actually executing
anything. This can be useful when integrating with another build tool.
Example:

```
```ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```ignore
```sh

@@ -280,7 +280,7 @@ need to have the source code for the standard library available, and at this
time the only supported method of doing so is to add the `rust-src` rust rustup
component:

```
```ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```ignore
```console

@@ -290,7 +290,7 @@ just forced to pass `--target` in one form or another.

Usage looks like:

```
```ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```ignore
```console

@@ -310,7 +310,7 @@ Using `-Z build-std` will implicitly compile the stable crates `core`, `std`,
`test` crate. If you're working with an environment which does not support some
of these crates, then you can pass an argument to `-Zbuild-std` as well:

```
```ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```ignore
```console

@@ -348,7 +348,7 @@ the tracking repository, and if it's not there please file a new issue!
The `timings` feature gives some information about how long each compilation
takes, and tracks concurrency information over time.

```
```ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```ignore
```sh

@bors
Copy link
Collaborator

bors commented Jan 31, 2020

☔ The latest upstream changes (presumably #7823) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Jan 31, 2020
@ehuss
Copy link
Contributor

ehuss commented Sep 3, 2020

I'm going to close, as I haven't heard back in a while. Feel free to reopen with the suggested changes, or open new PR if you're still interested.

@ehuss ehuss closed this Sep 3, 2020
bors added a commit that referenced this pull request Sep 4, 2020
fix mdbook test with ```ignore/text/sh/console

I revitalized the dead PR #7832

sorry about the delay

testing is just running `mdbook test` which now passes! Let me know if you want me to add mdbook test to ci as well!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants