Skip to content

Update ORCA Generator#33

Open
brockdyer03 wants to merge 36 commits intoOpenChemistry:masterfrom
brockdyer03:update_orca_generator
Open

Update ORCA Generator#33
brockdyer03 wants to merge 36 commits intoOpenChemistry:masterfrom
brockdyer03:update_orca_generator

Conversation

@brockdyer03
Copy link
Contributor

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

Signed-off-by: brockdyer03 <[email protected]>
Signed-off-by: brockdyer03 <[email protected]>
Signed-off-by: brockdyer03 <[email protected]>
Copy link
Contributor

@matterhorn103 matterhorn103 left a comment

Choose a reason for hiding this comment

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

Looks pretty good. Lot of work here, the generator will be hugely more useful after this, nice one.

As a general comment, I think a big concern with the generators is that they are one of the main things that people are likely to try and contribute to, so they should be as amenable to drive-by contributions as possible.

The number of files being added does make me somewhat concerned in that regard. But I think it's necessary for the complexity.

What you can (and should) do to help people is to add at least a brief docstring for every class. Doesn't have to be as thorough as some of the ones you write, doesn't always need an attributes block or what have you, but just a note to explain what it does and ideally why it exists would go a long way.

You also need to add the header to all the Python files.

[[default.rules]]
format.preset = "str_literal"
patterns = [
{ regexp = '\b(True)\b', caseSensitive = false },
Copy link
Contributor

Choose a reason for hiding this comment

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

Surely a boolean shouldn't be classed as a string literal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd be curious what a good alternative is, if there is one. I'd prefer to have them very clearly highlighted, and the str_literal preset is a nice bright red.

@brockdyer03
Copy link
Contributor Author

I am pretty sure I'm content with this as-is. The warning handling seems to be good, the syntax highlighting is mostly good, and I am happy with how the tabs work (for the most part).

Here's a preview of what a user could in theory see while using this version:
image

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