Small contribution

[xandrade]

Short description of change

Small contribution for readme file and adding tailing coma in dictionaries

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [x] I have read the docs/CONTRIBUTING.md document.
• [x] My code follows the code style of this project.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [x] I have read tests/README.md.
• [ ] I have added tests to cover my changes.
• [x] All new and existing tests passed.

What is the change?

What does it fix/add?

Test Configuration

• PLC Model
• PLC Firmware
• pylogix version
• python version
• OS type and version

[TheFern2]

Thanks for the PR. I don't how I feel about just adding commas, and rewording one sentence. This is the type of PR I would normally accept for a complete beginner new to OSS just to encourage them. I'd let @dmroeder be the ultimate judge for this PR.

[dmroeder]

I guess I'm indifferent on this one. If I were to have done this myself, I see it as 2 commits, readme update and trailing commas, but that is a style preference I suppose. The wording does sound a little better.

What would make it more compelling is having a commit that address the issue opened regarding python version mentioned in the readme.

[drbitboy]

Personally, I prefer this:

        self.CIPTypes = {0x00: (0, "UNKNOWN", '?')
                        ,0xa0: (88, "STRUCT", '<B')
                        ,0xc1: (1, "BOOL", '?')
                        ,0xc2: (1, "SINT", '<b')
                        ,...
                        ,0xda: (1, "STRING", '<B')
                        }

Although, tbf, that is made unnecessary by Python's choice to allow a trailing on the last element.

However, I would still put the closing } on a separate line.

OT: My daughter thinks the trailing final comma syntax thing has been added to a few languages to minimize output from git diff.

[TheFern2]

Tbh I'd just use auto formatting and call it a day. Life is too short to worry about formatting, but that's just me. If you open pylogix in pycharm there's way too many pep warnings but I've never worried about them too much since I know it might be Dustin's preferred style.

The only thing that really bugs me a little is the inconsistencies on naming some have camel case but most have pascal case. But changing the case at this point would be a huge breaking change that I'd rather not push.

[dmroeder]

Harsh but fair. I've learned a lot since starting this project. I'd do some things different today...

[TheFern2]

Oh I know I'm the same way. We learn throughout the years. I think being pythonic is important, but also introducing breaking changes to a well-known library is not a good idea.