avro icon indicating copy to clipboard operation
avro copied to clipboard

AVRO-3571: Shifted integer and long constants to constants.py

Open Diptonil opened this issue 2 years ago • 6 comments

The changes made are:

  • Code altered according to PEP 8 conventions. Files changed are: io.py, constants.py, schema.py, codecs.py, datafile.py and compatibility.py.
  • General constants INT_MIN_VALUE, INT_MAX_VALUE, LONG_MIN_VALUE & LONG_MAX_VALUE shifted from schema.py to constants.py for clarity and better organization.

(This is my first attempt to contribute to any open source project, not only this project. If I have made any errors or have misunderstood any step, please forgive me. I would be happy to adhere to any suggestions)

Jira

  • [x] My PR addresses the following Avro Jira issues and references them in the PR title. For example, "[AVRO-3571]: Python code refactor and rearrangement"
    • https://issues.apache.org/jira/browse/AVRO-3571
    • No dependencies are being added.

Tests

  • [x] My PR does not need testing because I have only moved certain constants from schema.py to constants.py and have refactored code to better adhere to the PEP 8 style guide.

Commits

  • [x] My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • [x] My commit doesn't add any functionality whatsoever.

Diptonil avatar Jul 07 '22 21:07 Diptonil

Hello!

(This is my first attempt to contribute to any open source project, not only this project. If I have made any errors or have misunderstood any step, please forgive me. I would be happy to adhere to any suggestions)

That's great, thanks for joining the community! You should be aware that we're a bit slow in reviewing PRs right now -- it's both summertime and working towards a 1.11.1 release. We'll get to this as soon as we can.

In the meantime, it looks like a lint failed with our build scripts... can you take a look at why this happens (and only with python 3.10?)

RyanSkraba avatar Jul 07 '22 22:07 RyanSkraba

Hello! I just enabled the Flake8 linter for my project. There are a lot of "line too long" warnings for every file. I'm assuming that I should leave that be? There are some other warnings as well - unused variables and imports. Would it be pertinent of me to clear off those warnings? Thank you. I understand if the review process is slow. That's totally fine.

Diptonil avatar Jul 07 '22 23:07 Diptonil

Don't worry about stylistic comments from flake8. The style for the Avro Python code is managed by black.

But feel free to clean up dead code such as unused imports.

kojiromike avatar Jul 07 '22 23:07 kojiromike

Thanks for the heads up. Will go through the code more extensively using black... Also, my last commit seems to have passed all workflows. Do review it whenever time permits and merge it if its is alright...

Diptonil avatar Jul 08 '22 05:07 Diptonil

I shall get back after properly scrutinizing the work soon.

Diptonil avatar Jul 08 '22 05:07 Diptonil

I resolved the issue with flake8 in AVRO-3807. If you run autoflake on your branch it should help clean up the conflicts.

kojiromike avatar Jul 20 '23 03:07 kojiromike