Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Move xkcd_dict.json to HomeDir and Download xkcd in WorkingDir #9

Merged
merged 3 commits into from
Jan 6, 2016
Merged

Conversation

Kickball
Copy link
Contributor

@Kickball Kickball commented Jan 6, 2016

I have solved requests 1 and 2 of Issue #6:

I've changed all of the logical to assume that xkcd_dict.json will be in the home dir.

I've changed the download functions to download the xkcd files to the working dir of the terminal.

Note:

This is tested to work on Ubuntu 14, however it may have issues with Windows due to use of / (and possible use of \ in Windows file system). I don't have a Windows machine to test with.

Should the above issue pose a problem we would have to check the operating system when the script is first run and use appropriate slashes.

I have solved requests 1 and 2 of Issue #6:

I've changed all of the logical to assume that xkcd_dict.json will be in the home dir.

I've changed the download functions to download the xkcd files to the working dir of the terminal.
@Kickball Kickball mentioned this pull request Jan 6, 2016
@tasdikrahman
Copy link
Owner

Looks good to me. But there would be a better way to create files in an OS independent way right #L37?

And can you confirm that you tested out all the methods listed here on the README on your machine

@Kickball
Copy link
Contributor Author

Kickball commented Jan 6, 2016

The newest commit (e11a0f9) should make the paths OS independent.

I will test all README methods listed soon.

@Kickball
Copy link
Contributor Author

Kickball commented Jan 6, 2016

The last commit (597430c) has been tested and all commands are confirmed to work on Ubuntu 14.

tasdikrahman pushed a commit that referenced this pull request Jan 6, 2016
Move xkcd_dict.json to HomeDir and Download xkcd in WorkingDir by @Kickball
@tasdikrahman tasdikrahman merged commit 810e58e into tasdikrahman:master Jan 6, 2016
@Kickball Kickball deleted the patch-1 branch January 6, 2016 18:56
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.

None yet

2 participants