Skip to content

Conversation

Shivang-Bhandari
Copy link

Updated the command to run the project locally using Python 3

@Shivang-Bhandari
Copy link
Author

@akshayaurora would be great if you could take a look.

@realslimshanky
Copy link
Member

realslimshanky commented Jul 21, 2017

@Shivang-Bhandari please specify the python version you are using.
I deployed your fix and running staticjinja on Python 3.5.2 (other versions might also) is throwing error while importing easywatch which is a currently an open issue with a temporary solution.
It's better to go with the solution which works i.e. Python 2 in this case.

@Shivang-Bhandari
Copy link
Author

Shivang-Bhandari commented Jul 22, 2017

@realslimshanky Python 3.6.0
If you feel we should stick to Python 2 then it's not a problem.

@realslimshanky
Copy link
Member

@Shivang-Bhandari If you have tested it on 3.6.0 specifically do mention in the README about it. Because other versions might fail to run the script.

@Shivang-Bhandari
Copy link
Author

@realslimshanky sure

README.md Outdated
@@ -12,6 +12,11 @@ To monitor your source directory for changes, and recompile files if they change
python build.py & python -m SimpleHTTPServer && fg
```

For Python 3.6.0 :
```
python build.py & python -m http.server && fg

Choose a reason for hiding this comment

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

This should work for all versions > 3.3

Its better to mention that, 3.6 was released recently, but this should work with relatively newer python 3 versions

Copy link
Author

Choose a reason for hiding this comment

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

@SanketDG in the above comments @realslimshanky said it was not working for him for version 3.5.2, it would be great if you could update me and I can make the necessary changes. :)

Choose a reason for hiding this comment

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

Sorry for the late response, but the above code should work for any versions > 3.3 since there have been no breaking changes after that.

So you can go ahead and change that

Copy link
Author

Choose a reason for hiding this comment

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

@SanketDG Done

Copy link
Author

Choose a reason for hiding this comment

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

Would be great if you could merge it
:)

@realslimshanky
Copy link
Member

@SanketDG would you like to have another look after the update?

@SanketDG
Copy link

@realslimshanky LGTM

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