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

several fixes, checked for portability #2

Merged
merged 6 commits into from
Mar 4, 2020
Merged

several fixes, checked for portability #2

merged 6 commits into from
Mar 4, 2020

Conversation

hannesdatta
Copy link
Contributor

  • Checked for portability on two other Windows systems
  • Removed any \gen files from tracking (these are GENERATED, hence, they need not be tracked)
  • Saved .gitkeep in the remaining (empty) dir structure to initialize directories
  • Added any remaining files that need not be tracked to .gitignore

Very good work, Rafael!

To do's:

  • the clean rule in makefile doesn't work - took it out from ALL
  • add link to Tex installation to your readme - alternatively, add an instruction page for installation to tilburgsciencehub via a PR on Git?
  • the \ in make look weird. really required? Hard for people to read.
  • Ensure compatibility w/ Linux and Mac; pending...

@rgreminger rgreminger self-assigned this Mar 4, 2020
@rgreminger rgreminger merged commit 4d48553 into rgreminger:master Mar 4, 2020
@rgreminger
Copy link
Owner

rgreminger commented Mar 4, 2020

Hi Hannes,

I just saw this now, probably should have held off with the release.. Thanks for the changes, they're very helpful! It definitely makes sense to use .gitkeep files to keep the folder structure, I was actually not aware of this option. The same goes with the clean rule. That one I'll have to fix anyways to make it platform independent, from what I understand it's the only part that won't work on other platforms than Windows.

But I'd remove the input folders, since with larger datasets this can eat up disk space quickly. For now added them in there from your PR, so it's consistent with the page (I've also opened an issue ).

The other things you mentioned I'll open separate issues for them. Will try to fix them in the coming days.
Best,
Rafael

@hannesdatta
Copy link
Contributor Author

hannesdatta commented Mar 5, 2020 via email

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.

2 participants