Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix video URL in README #242

Merged
merged 3 commits into from
Jul 5, 2022
Merged

Fix video URL in README #242

merged 3 commits into from
Jul 5, 2022

Conversation

bryanhonof
Copy link
Contributor

@bryanhonof bryanhonof commented Jul 4, 2022

Fixes #241

@bryanhonof bryanhonof requested a review from aherrmann July 4, 2022 20:29
@aherrmann
Copy link
Member

@bryanhonof the top-level README.md is generated. You need to edit the docstring here instead.

@bryanhonof
Copy link
Contributor Author

@aherrmann I had to make the changes in nixpkgs/nixpkgs.bzl. Is this by any chance duplicate functionality?

@aherrmann
Copy link
Member

I had to make the changes in nixpkgs/nixpkgs.bzl. Is this by any chance duplicate functionality?

Ah, good catch, I missed that. One generates the top-level README and the other generates the README for the core module. You can take a look at #181 and related PRs for context.

Thanks for the fix. You still have to run bazel run //docs:update-README.md to update the README and check-in the result.

@bryanhonof
Copy link
Contributor Author

bryanhonof commented Jul 5, 2022

My manual edits seem to be machine-like :).

rules_nixpkgs on  change-video-url 
❯ bazel run //docs:update-README.md
INFO: Analyzed target //docs:update-README.md (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //docs:update-README.md up-to-date:
  bazel-bin/docs/update-README.md
INFO: Elapsed time: 0.041s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Build completed successfully, 1 total action

rules_nixpkgs on  change-video-url 
❯ git status
On branch change-video-url
nothing to commit, working tree clean

@aherrmann
Copy link
Member

Hmm, CI is still unhappy about something

//docs:check-README.md                                                   FAILED in 0.0s

@bryanhonof
Copy link
Contributor Author

bryanhonof commented Jul 5, 2022

@aherrmann Also needed to run the action in core/. CI's happy now :).

Copy link
Member

@aherrmann aherrmann left a comment

Choose a reason for hiding this comment

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

Thank you!

@aherrmann aherrmann added the merge-queue merge on green CI label Jul 5, 2022
@mergify mergify bot merged commit 22a9b3c into master Jul 5, 2022
@mergify mergify bot deleted the change-video-url branch July 5, 2022 10:04
@mergify mergify bot removed the merge-queue merge on green CI label Jul 5, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Video in the README is no longer available
2 participants