Skip to content

draft 1 of README.md edits #8

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

Open
wants to merge 8 commits into
base: addepar/master
Choose a base branch
from

Conversation

adparvum
Copy link
Contributor

README.rst Outdated

To package and publish to PackageCloud
======================================
Run build.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpickey but put code in code blocks

README.rst Outdated
======================

Requirements:
- A Packagecloud.io key with Environment variable set to it, such as PACKAGE_CLOUD_KEY="PACKAGECLOUDKEY"
Copy link
Contributor

Choose a reason for hiding this comment

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

why the caps?

Copy link
Contributor

Choose a reason for hiding this comment

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

and that might as well be a link. i'd consider adding a link to our wiki as well but leaning against it as this fork could be made public.

Copy link
Contributor

@majuscule majuscule left a comment

Choose a reason for hiding this comment

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

LGTMBC

README.rst Outdated
======================

Requirements:
- A Packagecloud.io key with Environment variable set to it, such as package_cloud_key="packagecloudkey"
Copy link
Contributor

Choose a reason for hiding this comment

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

err, i meant Packagecloud.io and Environment, the variable does need to be all caps

Copy link
Contributor

Choose a reason for hiding this comment

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

also an environment

README.rst Outdated

Requirements:
- A Packagecloud.io key with Environment variable set to it, such as package_cloud_key="packagecloudkey"
- A clean branch
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think there's any real concept of a clean branch, what you mean to say here is a clean worktree.

@majuscule
Copy link
Contributor

Also I just realized that the README is actually .rst not .md, this seems like a decent cheatsheet for the formatting.

@majuscule
Copy link
Contributor

majuscule commented Dec 13, 2017

(meaning that the link formatting etc needs to be corrected, see https://thomas-cokelaer.info/tutorials/sphinx/rest_syntax.html#external-links)

README.rst Outdated
This package offers a mock ``pyldap`` library that can be used
for testing code relying on ``pyldap`` without having to configure
This package offers a mock ``pyldap`` library that can be used
for testing code relying on ``pyldap`` without having to configure
Copy link
Contributor

Choose a reason for hiding this comment

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

one more annoying nitpick, no reason to wrap text here (i.e. 80 chars or w/e), so this should be one line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants