Skip to content
This repository has been archived by the owner on Feb 9, 2021. It is now read-only.

README format from md to rst #288

Merged
merged 22 commits into from
Sep 19, 2018
Merged

Conversation

fragmuffin
Copy link
Contributor

addresses issue #287

@AppVeyorBot
Copy link

@coveralls
Copy link

coveralls commented Jul 20, 2018

Coverage Status

Coverage remained the same at 92.28% when pulling cdcea63 on fragmuffin:feature/readme-rst into 15e567f on dcowden:master.

@codecov-io
Copy link

codecov-io commented Jul 20, 2018

Codecov Report

Merging #288 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #288   +/-   ##
=======================================
  Coverage   92.79%   92.79%           
=======================================
  Files          10       10           
  Lines        2207     2207           
=======================================
  Hits         2048     2048           
  Misses        159      159

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6251ec0...1024b65. Read the comment docs.

@AppVeyorBot
Copy link

Build cadquery 1.0.64 failed (commit 511a192211 by @fragmuffin)

@fragmuffin
Copy link
Contributor Author

oh! I just noticed that PyPI can handle Markdown 🙄
Is that new?; I thought it only handled rst.
Not sure if this is a good change then... perhaps I should have stuck with md.

@westurner
Copy link
Contributor

Sphinx can also handle markdown; though roles and directives are not implemented for markdown, so it makes sense for README to be in RST so that it can easily be included in the docs verbatim.

Thanks!

@dcowden
Copy link
Owner

dcowden commented Jul 21, 2018

Ok so the consensus is that this is ready to merge?

@fragmuffin
Copy link
Contributor Author

fragmuffin commented Jul 22, 2018

@dcowden

There are also minor changes to:

  • Dockerfile
  • setup.py

But if there are any unforeseen issues with those, I'm sure they can be fixed easily in the next release.

@fragmuffin
Copy link
Contributor Author

fragmuffin commented Jul 22, 2018

Oh, also... have a look at the result before you merge it in:
https://github.com/fragmuffin/cadquery/tree/feature/readme-rst

Although it should technically be the same, it seems the :align: center and :width 350 px parameters for img tags aren't being rendered by github... everything else is the same.

I'll just play with that for a bit; see if I can get the formatting working...

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@fragmuffin
Copy link
Contributor Author

Hmph 😞. I've played around a bit with the images, but I've come around full circle.
It seems the attributes are being picked up when rendering
eg: see alt="Binder" for pill in I just want to try things out
But :align: and :width: are (mostly) ignored.

*mostly?
Both :width: and :align: attributes are rendered in the logo img tag at the top, but the align attribute is deprecated in HTML5 (see: github/markup#163).
The question remains: how to center align an image when rendered to HTML5?
However, further down in the readme, neither :width: nor :align: are rendered to the KiCad img tag.

I'm so confused 😕

Does anyone have:

@dcowden
Copy link
Owner

dcowden commented Jul 22, 2018

@fragmuffin the cadquery spinx docs render images. For example, quickstart:
https://github.com/dcowden/cadquery/blob/master/doc/quickstart.rst

For a more advanced case, the cq examples uses a custom directive (cqplot) that executes cq script and then creates an image and embeds it.

@fragmuffin
Copy link
Contributor Author

@dcowden Sphinx must be using a different renderer, because it seems to be working in other examples as well.
But it looks like the github markup rst img width being ignored is a known issue github/markup#295 but won't be fixed 😢.

I'll try the proposed :scale: fix tonight to see if that's any better.

@AppVeyorBot
Copy link

@fragmuffin
Copy link
Contributor Author

fragmuffin commented Jul 24, 2018

OK!, I'm so very done with this...
I gave up with image and figure and reverted any scaled images to use the .. raw:: html directive.

So the equivalent of:

.. image:: https://foo.com/bar.png

    :width: 100px
    :align: center

which doesn't render properly, ref: github/markup#295 ... is:

.. raw:: html

    <div align="center">
        <img src="https://foo.com/bar.png" width="100px" />
    </div>

Now... At this point I'm assuming this will render the same in PyPI as it does in github.
All link targets and image sources are absolute (same as before), so there shouldn't be any problems there.

@AppVeyorBot
Copy link

@fragmuffin
Copy link
Contributor Author

@westurner what do you think? is it time to merge?

README.rst Outdated
Thanks go to cadquery contributor hyOzd ( Altu Technology ) for the example!


KiCad uses cadquery to build high quality models of electrictronic components. (`https://github.com/KiCad/packages3D <https://github.com/KiCad/packages3D>`_)
Copy link
Contributor

Choose a reason for hiding this comment

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

'electrictronic'

README.rst Outdated

CadQuery has several goals:

* Build lD models with scripts that are as close as possible to how you'd describe the object to a human.
Copy link
Contributor

Choose a reason for hiding this comment

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

IDK what this is: "lD models"

README.rst Outdated

If you are familiar with jQuery, you will probably recognize several jQuery features that CadQuery uses:

* A fluent api to create clean, easy to read code
Copy link
Contributor

Choose a reason for hiding this comment

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

api -> API

README.rst Outdated

CadQuery scripts have several key advantages over OpenSCAD:

#. **The scripts use a standard programming language**, python, and thus can benefit from the associated infrastructure.
Copy link
Contributor

Choose a reason for hiding this comment

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

python -> Python

@westurner
Copy link
Contributor

Conda install instructions would be great, too:

conda install -c conda-forge freecad notebook cadquery?

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@fragmuffin
Copy link
Contributor Author

@westurner good suggestions ;)
I've added the conda instruction in cdcea63 (which will publish the installation.html page) ... I've simplified it to:

conda install -c conda-forge cadquery

Although I didn't test it...
@dcowden @jmwright @adam-urbanczyk : Is that the recommended way to install cadquery using anaconda?; or do you have to explicitly reference FreeCAD as well when installing?

@AppVeyorBot
Copy link

@adam-urbanczyk
Copy link
Contributor

@fragmuffin it should be

conda install -c conda-forge cadquery

This works, but does not install all dependencies (e.g. FreeCad). So I have to conclude that the package is currently broken. Does anyone remember how was the conda-forge recipe prepared?

@adam-urbanczyk
Copy link
Contributor

adam-urbanczyk commented Sep 18, 2018

Adding for reference: conda-forge/cadquery-feedstock#2

Short memory: #169

@fragmuffin
Copy link
Contributor Author

@adam-urbanczyk excellent, thanks.
So I think this is ready to merge... @dcowden @jmwright what do you think?

@jmwright
Copy link
Collaborator

@fragmuffin @adam-urbanczyk

I have not used rst before. I'm assuming I'll have to learn its new markup format?

If the broken conda line isn't a showstopper, I think this is fine to merge.

@fragmuffin
Copy link
Contributor Author

fragmuffin commented Sep 19, 2018

@jmwright

I'm assuming I'll have to learn its new markup format?

Yes, it's a little different to markdown, but it has the same sentiment; the non-formatted text still gives a sense of hierarchy and format so it's easy to read.
I highly recommend learning it, especially when coding sphinx with Python, which cadquery does.

I often get markups mixed up, so I use an online editor for basic verification of rst format.
However, as this PR has demonstrated, there are nuances with different renderers that can cause issues.
But I think we've accurately assessed that the benefits of rst outweigh the negatives, in this case.

I've added a check to TravisCI that should fail a bad edit to the README.rst

If the broken conda line isn't a showstopper, I think this is fine to merge.

It is an issue, but out of scope for this PR

@jmwright
Copy link
Collaborator

@dcowden Are you ok with this being merged?

@dcowden
Copy link
Owner

dcowden commented Sep 19, 2018

@jmwright @fragmuffin definitely!

@jmwright jmwright merged commit 938261b into dcowden:master Sep 19, 2018
@jmwright
Copy link
Collaborator

@fragmuffin @westurner @adam-urbanczyk Thanks for the work on this.

@westurner
Copy link
Contributor

westurner commented Sep 19, 2018 via email

@westurner
Copy link
Contributor

westurner commented Sep 19, 2018 via email

@westurner
Copy link
Contributor

westurner commented Sep 19, 2018 via email

@jmwright
Copy link
Collaborator

@westurner The images show up for me.

@westurner
Copy link
Contributor

westurner commented Sep 19, 2018 via email

README.rst Show resolved Hide resolved
doc/installation.rst Show resolved Hide resolved
@fragmuffin
Copy link
Contributor Author

The showcase.gif link is still an issue.
I've made #294 to fix it.

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

Successfully merging this pull request may close these issues.

8 participants