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

Added negative check and is_percentage validator tests #83

Merged

Conversation

dgmouris
Copy link
Contributor

Add a check in the is_percentage validator so that it doesn't allow negative percentages.

This ensures that when the percentage unit is validated, that it can't be negative.

#75

PR Checklist:

  • All new features have been tested
  • All new features have been documented
    I'm unsure if this check needs documentation, but guidance on whether or not it does is greatly appreciated.
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@dgmouris
Copy link
Contributor Author

Hey when I run tox -e towncrier I'm getting an Exception of Can't find your project :( which I'm a bit confused about.

If someone could help me out a bit on this that would be fantastic, (as well if you do I'll open a PR to help with the contributing docs as this might be a wall for some folks).

Thanks for taking this PR into consideration.

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

The code looks good (although I'll need to wait to see the tests pass in CI);

The test failure you're currently seeing is because you need to add a new file to the changes directory. Essentially you (as the end developer) shouldn't be running tox -e towncrier; you only need to run tox -e towncrier-check. We (the maintainers) run towncrier as part of the release process, which generates the final release notes.

By running town crier yourself, you've deleted an old (unrelated) release note, and CI is noticing that there aren't any release notes.

@dgmouris dgmouris force-pushed the negative-percent-check-validator branch from 039b026 to f35a19a Compare October 19, 2020 01:11
@dgmouris
Copy link
Contributor Author

@freakboy3742 Thank you so much for the information on this.

I might add some information to the contributing page to add what I got stuck up on if that's alright:)

@dgmouris
Copy link
Contributor Author

As minor request. I'm participating in hacktoberfest and I was wondering if you could put hacktoberfest-accepted, totally okay if you don't want to or can't.

I do want to keep on participating project (beeware as whole) as you folks are doing really neat awesome work:)

@dgmouris dgmouris requested a review from freakboy3742 October 22, 2020 22:57
Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

Looks good - thanks for the PR!

Unfortunately, we're not going to turn on the Hacktoberfest tag - while we definitely appreciate the contribution, Hacktoberfest is always a magnet for the worst kind of drive-by contributions, which creates a huge burden for us as maintainers. This year it has been especially bad, and the hacktoberfest tag serves as a searchable magnets of these contributions.

@freakboy3742 freakboy3742 merged commit cbf974b into beeware:master Oct 24, 2020
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