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

Fixed test_bibtex unit test, updated CONTRIBUTING.md #525

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

dlebedinsky
Copy link

Fixes #395.

Description

the test_bibtex function in the test module was attempting to read a file that did not exist. I updated it to read the real file name, reran the test, and it worked.
I also updated CONTRIBUTING.md to address some pitfalls that I ran into when I tried to submit my first pull request yesterday. Hopefully this will prevent other newcomers from making the same mistakes I did!

Checklist

  • [ x] Check that the base branch is set to develop and not main.
  • [x ] Ensure that the documentation will be consistent with the code upon merging.
  • [x ] Add a line or a few lines that check the new features added.
  • [x ] Ensure that unit tests pass.
    If you don't have a premium proxy, some of the tests will be skipped.
    The tests that are run should pass without raising
    MaxTriesExceededException or other exceptions.

Copy link
Collaborator

@arunkannawadi arunkannawadi left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution.

@dlebedinsky
Copy link
Author

I just pushed another commit, in which I added a unit test for freeproxy. It is the only proxy I have, and I assume this is the case for many others, so I find it helpful. Do you see it? I assumed I would have to open a separate pull request, but I'm not sure now.

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