-
Notifications
You must be signed in to change notification settings - Fork 17
Resolve issues: #24, #64, #82, #118, other fixes #148
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
base: main
Are you sure you want to change the base?
Changes from all commits
1c115c8
96290e5
c03c63f
3c1ed75
c7d2ab3
755454c
9ac9a08
cede48c
dc6d8d2
938bb11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
|
|
||
| # These owners will be the default owners for everything in the repo. | ||
|
|
||
| * @cacraigucar @climbfuji @dustinswales @grantfirl @mattldawson @mkavulich @mwaxmonsky @nusbaume @peverwhee @MarekWlasak @svahl991 @ss421 | ||
| * @cacraigucar @climbfuji @dustinswales @grantfirl @mattldawson @mkavulich @nusbaume @peverwhee @MarekWlasak @svahl991 @ss421 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would remove @mattldawson as well, as I don't believe he is with NCAR anymore. I can look for an alternative NCAR chemistry representative in case we need one for future chemistry-related name changes/additions (which I can always add to the CODEOWNERS file in a later PR). |
||
|
|
||
| # Order is important. The last matching pattern has the most precedence. | ||
| # So if a pull request only touches javascript files, only these owners | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -64,26 +64,20 @@ def validate_xml_file(filename, schema_file, logger, error_on_noxmllint=False): | |||||
| """ | ||||||
| # Check the filename | ||||||
| if not os.path.isfile(filename): | ||||||
| raise ValueError(f"validate_xml_file: Filename, '{filename}', does not exist") | ||||||
| if not os.access(filename, os.R_OK): | ||||||
| raise ValueError(f"validate_xml_file: Cannot open '{filename}'") | ||||||
| raise FileNotFoundError(f"validate_xml_file: Filename, '{filename}', does not exist") | ||||||
| if not os.path.isfile(schema_file): | ||||||
| raise ValueError(f"validate_xml_file: Cannot find schema file {schema_file}") | ||||||
| if not os.access(schema_file, os.R_OK): | ||||||
| emsg = "validate_xml_file: Cannot open schema, '{}'" | ||||||
| raise ValueError(emsg.format(schema_file)) | ||||||
| raise FileNotFoundError(f"validate_xml_file: Cannot find schema file {schema_file}") | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might put quotes around
Suggested change
|
||||||
| if _XMLLINT is not None: | ||||||
| if logger is not None: | ||||||
| lmsg = "Checking file {} against schema {}" | ||||||
| logger.debug(lmsg.format(filename, schema_file)) | ||||||
| logger.debug(f"Checking file {filename} against schema {schema_file}") | ||||||
| cmd = [_XMLLINT, '--noout', '--schema', schema_file, filename] | ||||||
| result = call_command(cmd, logger) | ||||||
| return result | ||||||
| lmsg = "xmllint not found, could not validate file {}" | ||||||
| lmsg = f"xmllint not found, could not validate file {filename}" | ||||||
| if error_on_noxmllint: | ||||||
| raise ValueError("validate_xml_file: " + lmsg.format(filename)) | ||||||
| raise ImportError("validate_xml_file: " + lmsg) | ||||||
| if logger is not None: | ||||||
| logger.warning(lmsg.format(filename)) | ||||||
| logger.warning(lmsg) | ||||||
| return True # We could not check but still need to proceed | ||||||
|
|
||||||
| ############################################################################### | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look how up to date we are!