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

Another way to check if pass the test #66

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

Another way to check if pass the test #66

wants to merge 14 commits into from

Conversation

LuckyPigeon
Copy link

Sometimes I use py.test got failed, so I find out that I can use pip setup.py install, if install successfully and the outcome is the same as I imagine, then I can suggest that will pass the py.test.

@depau
Copy link
Collaborator

depau commented Mar 18, 2018

What the hell is this? Have you really included an entire virtualenv in the pull request?
Also the comments you've added are extremely misleading.

Copy link
Collaborator

@depau depau left a comment

Choose a reason for hiding this comment

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

This is garbage.

@LuckyPigeon
Copy link
Author

Oh, that's mean. I'm sorry about that, thank you for point out the mistake I have made, I will change it once I get home. But this PR is not a garbage. Have you make the changes, but when you use pytest, it shows 0 item collected in 0.01second? And that was happened last time I testing the project, since I was new to git, so I commit a lot of time, and I am sorry about that. But that's also the reason why I found out that using module install can also test if it's wrong with the changes I have made.

@LuckyPigeon
Copy link
Author

And thank you mention that I have to delete the virtualenv file, I think I would like to put that on contributing document, it allows newbie like me understand how to make a change in a right way.

@LuckyPigeon
Copy link
Author

I make the number of file correct and pull another request, I will close this PR, thanks for teaching again.

@PrabhanshuAttri
Copy link
Collaborator

PrabhanshuAttri commented Mar 18, 2018

@LuckyPigeon Please run git status before committing. Also, take a pull from master branch before making changes. This can avoid merge conflicts.

@depau Please be polite and welcoming to new contributors. I understand this might become frustrating but we believe in Open Code of Conduct or see CODE_OF_CONDUCT. This will result in pragmatic and productive conversations.

@LuckyPigeon
Copy link
Author

@PrabhanshuAttri Thanks for sharing some tutorial, it's very helpful.

@PrabhanshuAttri
Copy link
Collaborator

@LuckyPigeon and @depau you can join our slack channel as well.

Try this: git tutorial game

@LuckyPigeon LuckyPigeon reopened this May 5, 2018
.travis.yml Outdated
@@ -28,4 +28,4 @@ deploy:
password: $PYPI_PASSWORD
on:
branch: master
condition: $SHOULD_DEPLOY = 1
condition: $SHOULD_DEPLOY = 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please undo this change. Here is travis guideline for on condition deploy.

CONTRIBUTING.md Outdated

6. Commit your changes and push your branch to GitHub::
```shell
$ sudo pip install PyBeacon
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will install PyBeacon from PyPi, not from local repo after making changes. Remove this

@@ -111,6 +112,7 @@ def encodeUid(uid):
ret = []
for i in range(0, len(uid), 2):
ret.append(int(uid[i:i+2], 16))
"""Python's final character"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change this to Final byte stream character in Python

@@ -119,6 +121,7 @@ def uidIsValid(uid):
"""UID Validation."""
if len(uid) == 32:
try:
"""Must not be alpha except ABCDEF"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change this to UID characters must belong to base 16

@@ -170,6 +173,7 @@ def decodeUrl(encodedUrl):
"""
decodedUrl = schemes[encodedUrl[0]]
for c in encodedUrl[1:]:
"""Unprintable character"""
Copy link
Collaborator

@PrabhanshuAttri PrabhanshuAttri May 17, 2018

Choose a reason for hiding this comment

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

These are just not the unprintable chars. Please read about Eddystone URL and UID here and here. Remove this comment. Also, update your branch.

@LuckyPigeon
Copy link
Author

@PrabhanshuAttri When I saw the PR #67 , I have already push the update changes, is there any way to move this push to another branch?

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.

None yet

3 participants