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

Allow cleaning of examples #94

Merged
merged 1 commit into from
Jul 16, 2020

Conversation

aakatz3
Copy link
Contributor

@aakatz3 aakatz3 commented Jul 16, 2020

build_examples supports cleaning examples and intelligently will detect new examples.

SUGGESTION: When merging into dev, require

build_examples.py clean

and then only build when merging into master branch

This should allow easier merges, especially from windows, and I am using it as part of a resolution to one of the requests in #17

build_examples supports cleaning examples and intelligently will detect new examples.

SUGGESTION: When merging into dev, require
```
build_examples.py clean
```

and then only build when merging into master branch
@formatc1702
Copy link
Collaborator

This looks like a really useful improvement! Thanks!

@formatc1702 formatc1702 merged commit 861380d into wireviz:dev Jul 16, 2020
@formatc1702
Copy link
Collaborator

Any idea how it chooses the order of the files? Any quick way to fix this?
See my console output when I run the script:

(venv) wireviz $ python build_examples.py 
../../examples/ex09.yml
../../examples/ex08.yml
../../examples/ex01.yml
../../examples/ex03.yml
../../examples/ex02.yml
../../examples/ex06.yml
../../examples/ex07.yml
../../examples/ex05.yml
../../examples/ex04.yml
../../examples/demo01.yml
../../examples/demo02.yml
../../tutorial/tutorial08.yml
../../tutorial/tutorial02.yml
../../tutorial/tutorial03.yml
../../tutorial/tutorial01.yml
../../tutorial/tutorial04.yml
../../tutorial/tutorial05.yml
../../tutorial/tutorial07.yml
../../tutorial/tutorial06.yml

@formatc1702
Copy link
Collaborator

formatc1702 commented Jul 16, 2020

Also, I'm not sure cleaning up (by deleting) the examples pre-merge is the best course of action, because then the readme of the dev branch would include references to nonexistent files; same for the tutorial and example galleries.

What I've been doing is simply discarding any changes I've made to the output files before commiting anything to dev.

As the project matures, there probably will be less and less changes that affect the output files, but who knows.

@formatc1702
Copy link
Collaborator

formatc1702 commented Jul 17, 2020

Perhaps one option would be to keep examples out of the repo completely (by .gitignoreing the entire directory), and curating a separate wireviz-gallery repo that can be rebuilt and updated at will. The WireViz readme could directly link to those files on the other repo. This would avoid links to missing files.
@aakatz @kvid how do you feel about this proposal?

@kvid
Copy link
Collaborator

kvid commented Jul 17, 2020

Personally, I'm actively using the input files together with the output files (those that don't depend on the Graphviz version) for regression testing to verify that my commits either don't change any output or produce the expected output diff. That's also why I suggested issue #63

I would therefore prefer to keep these files unless the regression testing can be secured some other way. Why not change the clean action to do git checkout -- {generated files} or something similar?

@kvid
Copy link
Collaborator

kvid commented Jul 17, 2020

If build_examples.py is changed to write generated files (and perhaps a copy of the YAML file) to a _built subfolder unless some action argument specify to overwrite the committed files, then it will be easy both to compare any output differences side by side and to ignore the subfolder when committing. What do you think?

Two advantages by copying the YAML file to the subfolder first:

  • Avoid changes to the Harness class.
  • Avoid bad references from the readme files.

Copy link
Collaborator

@kvid kvid left a comment

Choose a reason for hiding this comment

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

Consider sorting the list returned from os.listdir() (called 4 places?) as suggested here: https://stackoverflow.com/questions/4813061/non-alphanumeric-list-order-from-os-listdir
See unsorted output in #94 (comment)

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.

3 participants