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

Contributing #109

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

EvenBetterClub
Copy link

Issues #108 - I was unable to get a successful build working so could contribute - Have written a more details setup guide - but it still needs work to be complete - can anyone help?

@rickhull
Copy link

rickhull commented May 13, 2020

I got tests working on NixOS 20.03 with less work.

  # List packages installed in system profile. To search, run:
  environment.systemPackages = with pkgs; [
    # ...
    # added for diagrams
    python3
    poetry
    graphviz
];
git clone $DIAGRAMS
cd diagrams
poetry shell
python -m unittest tests/*.py

Copy link
Contributor

@louisguitton louisguitton left a comment

Choose a reason for hiding this comment

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

Thanks for your PR, it helped me fill in the blanks while getting setup. I'm on MacOS and here are the steps I follow to set myself up, if that can be helpful to anybody:

Dev environment

You need poetry installed and a virtual environment (both are not specific to this project so I'm not including it here), and then in your diagrams folder run:

poetry install
python -m unittest tests/*.py

Contributing environment

go get github.com/mingrammer/round
brew cask install inkscape
./autogen.sh

Comment on lines +69 to +72
* Black (used by autogen.sh)
```shell
pip3 install black
```
Copy link
Contributor

Choose a reason for hiding this comment

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

you can let poetry handle this by setting up your dev environment with

poetry install

Comment on lines +87 to +90
* Convert
```shell
pip install convert
```
Copy link
Contributor

@louisguitton louisguitton Jun 8, 2020

Choose a reason for hiding this comment

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

you saved me probably at least (:trollface: ) 15 minutes on this one 🙏 .
The error message if you don't do this is:

→ ./autogen.sh
image magick is not installed

A quick search in the codebase shows that it's fixed by installing convert or ImageMagick, which I did with

brew install imagemagick


# WARNING - 19 Apr 2020 - The below creates an environment where you can successfully run the tests

# However it still needs testing again; and also needs worked example on actually contributing 'value' and not putting noise back into the repository!
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# However it still needs testing again; and also needs worked example on actually contributing 'value' and not putting noise back into the repository!
# However, it still needs testing again; and also needs worked example on actually contributing 'value' and not putting noise back into the repository!


# However it still needs testing again; and also needs worked example on actually contributing 'value' and not putting noise back into the repository!

Whilst using Diagrams is easy and some folks will find setting up and extending Diagrams easy - for others with Python, Bash and Go dependancies it is harder...
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Whilst using Diagrams is easy and some folks will find setting up and extending Diagrams easy - for others with Python, Bash and Go dependancies it is harder...
While using Diagrams is easy and some folks will find setting up and extending Diagrams easy - for others with Python, Bash and Go dependencies it is harder...


So a worked guide to setting up a VM with linux so you can contribute to Diagrams.

Thanks: Thanks to ViktorOrda for assist on getting this to work.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Thanks: Thanks to ViktorOrda for assist on getting this to work.
Thanks: Thanks to @ViktorOrda for assist on getting this to work.


Whilst using Diagrams is easy and some folks will find setting up and extending Diagrams easy - for others with Python, Bash and Go dependancies it is harder...

So a worked guide to setting up a VM with linux so you can contribute to Diagrams.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
So a worked guide to setting up a VM with linux so you can contribute to Diagrams.
So, a worked guide to setting up a VM with Linux so you can contribute to Diagrams.

Comment on lines +123 to +125
and all the tests should pass.


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
and all the tests should pass.
and all the tests should pass.

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.

4 participants