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

Update README add installation instructions #44

Closed
wants to merge 10 commits into from

Conversation

Redsandro
Copy link
Contributor

@Redsandro Redsandro mentioned this pull request May 6, 2022
3 tasks
@mara004
Copy link
Contributor

mara004 commented May 6, 2022

I think that's a helpful addition. One can't expect that anyone is familiar with Python tooling.

@Redsandro
Copy link
Contributor Author

(You may notice a couple more commits. The problem is that I can't get the code blocks to render correctly even though I believe I use the correct markdown. Perhaps someone with merge permission can fix it if it still doesn't render correctly after merge.)

@MerlijnWajer
Copy link
Collaborator

I think this is helpful, but I think it might also make sense to mention that the tagged releases are available on https://pypi.org/project/archive-pdf-tools/ - which means people can run pip install archive-pdf-tools==1.4.14 for example.

BTW: I have been working on making the read the docs version better, which is here: https://archive-pdf-tools.readthedocs.io/en/latest/ - it's basically a more comprehensive version of the readme (some of it is copy paste)

I think having an installation chapter there would also be good.

@Redsandro
Copy link
Contributor Author

it might also make sense to mention that the tagged releases are available on

Sounds good. Would you like me to amend the PR or are you asking for opinions before you amend the PR?

I have been working on making the read the docs version better, which is here:

Where can this be amended? Is it automatically generated from the docs/ folder?

@MerlijnWajer
Copy link
Collaborator

Right, that's correct, that is in docs/. And yes, you can add it to the PR here if you're up for it.

@Redsandro
Copy link
Contributor Author

you can add it to the PR here if you're up for it.

Ok, that's about it. Sorry for the many commits. I didn't know RST, thought it was similar to MD. But now it works.

@MerlijnWajer
Copy link
Collaborator

Thanks, mind if I squash the commits into one?

@Redsandro
Copy link
Contributor Author

Not at all, do what you want. 🙂


First install dependencies. For example, in Ubuntu::


Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason you are always leaving 2 lines empty? I think 1 should be quite sufficient...

Copy link
Collaborator

Choose a reason for hiding this comment

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

I can fix that up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any reason

When in Rome, do as the Romans do. 🙂 I looked at the first existing code block I found (under "Usage")

@@ -61,6 +61,50 @@ For JBIG2 compression:
* `jbig2enc <https://github.com/agl/jbig2enc>`_ for JBIG2 compression (and PyMuPDF 1.19.0 or higher)


Installation
------------

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if it makes sense to duplicate the whole section. I personally wouldn't do that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's not too bad, if anything, maybe the README ought to refer to the docs for specifics.

@MerlijnWajer
Copy link
Collaborator

I've merged this to master, thanks

@Redsandro Redsandro deleted the patch-2 branch May 8, 2022 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants