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

Start of urlchecker python module! #2

Merged
merged 10 commits into from
Mar 20, 2020
Merged

Start of urlchecker python module! #2

merged 10 commits into from
Mar 20, 2020

Conversation

vsoch
Copy link
Collaborator

@vsoch vsoch commented Mar 20, 2020

This pull request will add automated testing to the current master branch (which is a slightly older version, I realized I needed to put something there). I've worked on the following:

  • the module is organized in a way that I hope is intuitive (described in the README.md) meaning that the client folder serves the entrypoint (client), the core folder contains the same core functions (that are agnostic to cloning, setup, etc.) and the main folder contains helper functions for different integrations (e.g., GitHub).
  • the version.py file handles metadata and dependencies
  • the README.md has a quick walk through to run locally on a folder, or repository. This needs to be replaced by pretty / better docs, but I wanted to have it somewhere for now.
  • GitHub actions are used to run tests
  • a Dockerfile provides a container build, in case anyone wants to use in a container.
  • a CHANGELOG is added to track major changes corresponding with pypi releases

That's it! I'll update this branch to likely address testing failures.

Signed-off-by: vsoch <vsochat@stanford.edu>
Signed-off-by: vsoch <vsochat@stanford.edu>
Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch
Copy link
Collaborator Author

vsoch commented Mar 20, 2020

@SuperKogito we need to disable travis checks here, this isnt a travis.yml to check (and I'm purposefully staying away because I'm unhappy with the CI across various projects). Could that be possible?

Signed-off-by: vsoch <vsochat@stanford.edu>
Signed-off-by: vsoch <vsochat@stanford.edu>
Signed-off-by: vsoch <vsochat@stanford.edu>
@SuperKogito
Copy link
Member

I would have expected travis-ci to be more useful here? why are you not satisfied with it? what would be the alternative?

@vsoch
Copy link
Collaborator Author

vsoch commented Mar 20, 2020

Ah, well in support of not wanting to re-write all the testing, let me see if I can add a .travis.yml.

Signed-off-by: vsoch <vsochat@stanford.edu>
@SuperKogito
Copy link
Member

well you can drop it if it is that much of a hustle. I can take care of it if you want ;)

Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch
Copy link
Collaborator Author

vsoch commented Mar 20, 2020

I can do it! I'm too stubborn to not complete the work. Give me another few shots :)

@SuperKogito
Copy link
Member

lol :P take your time, no pressure ;)

Signed-off-by: vsoch <vsochat@stanford.edu>
Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch
Copy link
Collaborator Author

vsoch commented Mar 20, 2020

There we go! All green! It was really rather stupid of me, I just forgot to update the way that it's installed, and then I had two bugs (a spelling typo and giving an int to subprocess). I suppose most failures are for reasons like that. We should be good to go now!

You can see in the README how I was running this locally - it's going to be super easy to wrap this client into a github action that just is a Dockerfile with a shell script to run the same command. What is your pypi username? I can add you to the package as a maintainer. And then after this is merged we can do a release there for 0.0.1 (and do you want to release on GitHub as well?)

Once it's released on pypi, it should be fairly straight forward to update the GitHub actions! I can do this tomorrow if you like, I'm having a lot of fun :)

One quick question - is there any reason to have travis run twice for a PR? The testing already takes a bit of time, and then the user (potentially) needs to wait 1.5X that. Maybe PRs should just be tested, and then merges to master / pushes to random branches ignored?

@vsoch
Copy link
Collaborator Author

vsoch commented Mar 20, 2020

Merge when you are ready, and let me know how you'd like to move forward! I'd like to suggest:

  • adding you on pypi
  • I'll release 0.0.1 there from master here
  • I'll refactor the action (and open a PR) this weekend to test

Probably no more work for today (other than a release if we merge), almost dinner time! :)

@SuperKogito
Copy link
Member

great work Vanessa! as usual 👏 I love the way you structured the project and the separations of some features 👍 . Everything seems great, so you can merge!
My username is also SuperKogito. Your plan seems good, I will let you steer now and try to figure the new structure in the upcoming days :)) Feel free to make all the changes you judge necessary. I am glad that you are enjoying this <3 cuz it is never fun when one is doing it alone. I truly think that without your contributions, I might of gave up on the tool or at least, I wouldn't have been as active as I was :)

Well you make a good point about travis running twice. Technically, travis is running once on the new branch and once on the PR. I think we should drop the branch run and settle for one run for the PR? if you think it is slow, we can use pytest-xdist (like I did initially) but the logs won't be as detailed as now :P

@vsoch
Copy link
Collaborator Author

vsoch commented Mar 20, 2020

Sounds good! I'll open an issue to address travis. Merging!

@vsoch vsoch merged commit 93d7fcb into master Mar 20, 2020
@vsoch
Copy link
Collaborator Author

vsoch commented Mar 20, 2020

https://pypi.org/project/urlchecker/0.0.1/ and you are added!

@vsoch vsoch deleted the start-of-module branch March 20, 2020 23:43
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

2 participants