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

Support admin render_readme with Cargo.toml without optional readme field #3970

Merged
merged 3 commits into from
Oct 5, 2021

Conversation

nipunn1313
Copy link
Contributor

@nipunn1313 nipunn1313 commented Oct 1, 2021

Depends on #3969

(Because of community/community#4477 (comment) - github ends up rendering both diffs together. Go to the commits tab and just look at the most recent commit to review).

https://doc.rust-lang.org/cargo/reference/manifest.html#the-readme-field

@Turbo87 Turbo87 added A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works labels Oct 1, 2021
@bors
Copy link
Contributor

bors commented Oct 1, 2021

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

src/admin/render_readmes.rs Outdated Show resolved Hide resolved
@Turbo87
Copy link
Member

Turbo87 commented Oct 1, 2021

Now that I read the docs on this a little more closely I'm wondering how we are handling readme = false, because I guess it wouldn't be able deserialize to Option<String>, right? do we just panic in that case? 😱

@Turbo87
Copy link
Member

Turbo87 commented Oct 1, 2021

I've rebased the branch btw :)

@nipunn1313
Copy link
Contributor Author

yep you're right. There's more to the spec that's not supported. Let me see what I can do.
At the least - this is an improvement (supports more cases than before).

@nipunn1313
Copy link
Contributor Author

nipunn1313 commented Oct 1, 2021

#3971 adds support for out-of-order tar files
That unblocks #3972 which adds support for README.txt and README as well as readme = false
Together they unblock #3973 - which supports relative links in READMEs from non-repo-root packages. Technically it only logically depends on #3971 - but for ease of code rebase I stacked them in this order.

I know there are timezones and whatnot between us, so feel free to update/change these PRs to get as much done as you'd like.

@Turbo87
Copy link
Member

Turbo87 commented Oct 5, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Oct 5, 2021

📌 Commit 824055f has been approved by Turbo87

bors added a commit that referenced this pull request Oct 5, 2021
Support admin render_readme with Cargo.toml without optional readme field

Depends on #3969

(Because of community/community#4477 (comment) - github ends up rendering both diffs together. Go to the commits tab and just look at the most recent commit to review).

https://doc.rust-lang.org/cargo/reference/manifest.html#the-readme-field
@bors
Copy link
Contributor

bors commented Oct 5, 2021

⌛ Testing commit 824055f with merge d9a57fc...

@bors
Copy link
Contributor

bors commented Oct 5, 2021

💥 Test timed out

@Turbo87
Copy link
Member

Turbo87 commented Oct 5, 2021

probably caused by https://www.githubstatus.com/incidents/bdbzpz7qxmbx

@bors retry

@bors
Copy link
Contributor

bors commented Oct 5, 2021

⌛ Testing commit 824055f with merge 84495e3...

@bors
Copy link
Contributor

bors commented Oct 5, 2021

☀️ Test successful - checks-actions
Approved by: Turbo87
Pushing 84495e3 to master...

@bors bors merged commit 84495e3 into rust-lang:master Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants