Skip to content

Update parse usernames process #71

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

Merged
merged 4 commits into from
Oct 22, 2023
Merged

Conversation

SimonMolinsky
Copy link
Contributor

#70

I've created a function outside ProcessIssues() class for parsing usernames. Additionally, I've written a simple unit test. I didn't check the process of updating webpage data because I don't have a Github Token for it (maybe we need some toy repository to test this package?).

🐢

@lwasser
Copy link
Member

lwasser commented Oct 22, 2023

@SimonMolinsky thank you for this - i'll have a look at it locally now!!

and yay we have our first test !! i've wanted to add tests i just haven't had time. thank you !!!
as far as a test repository. we can totally have something like that. i guess we need to think about whether it's needed. in the other repo i think the token is for opening the PR. BUT perhaps you are right there is an api token for accessing github.

you should be able to run this all locally - so i'd love to understand what didn't run or what was confusing about the workflow so i can add documentation. so far it's only been me working on it. Having fresh eyes is really useful.

@lwasser
Copy link
Member

lwasser commented Oct 22, 2023

amazing! it runs locally. i'm going to push a small enhancement that then validates package names using the same cleanup step you used for names. i noticed astartes has markdown in the name!! but let's merge this now and then we can move forward with that other cleanup step!!

@SimonMolinsky i've always used pytest to run tests. in this case how would we run the tests?
many thanks

@lwasser lwasser merged commit 791a624 into pyOpenSci:main Oct 22, 2023
@lwasser
Copy link
Member

lwasser commented Oct 22, 2023

@all-contributors please add @SimonMolinsky for code, review

@allcontributors
Copy link
Contributor

@lwasser

I've put up a pull request to add @SimonMolinsky! 🎉

@SimonMolinsky
Copy link
Contributor Author

Two new things appeared in this PR, so I will start with the first:

you should be able to run this all locally - so i'd love to understand what didn't run or what was confusing about the workflow so i can add documentation. so far it's only been me working on it. Having fresh eyes is really useful.

Ok, I didn't grasp it because I don't know what exactly is the output of each process, and if I can run processes separately. I will experiment with it and tell you what is/was confusing (or rather - missing) in README.

@SimonMolinsky i've always used pytest to run tests. in this case how would we run the tests?

I've used unittest because I don't see pytest as a dependency. If you wish, I can change the code and add pytest to dependencies, but in another Issue I suppose :) I prefer pytest too :)

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