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.

Sign up for GitHub

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

Jump to bottom

Tag more filter examples as optional #263

Merged
merged 3 commits into from
Mar 13, 2020
Merged

Tag more filter examples as optional #263

merged 3 commits into from
Mar 13, 2020

Conversation

ml-evs
Copy link
Member

@ml-evs ml-evs commented Mar 6, 2020 •
edited
Loading

Closes #262, if desired...

  • tag more bullet-pointed filter examples as OPTIONAL on the same line where it has already been indicated as optional in the text.
  • re-tagged one :filter: as a :filter-fragment:
  • fixed a spelling typo

rartino
rartino reviewed Mar 10, 2020
View reviewed changes
optimade.rst Outdated Show resolved Hide resolved
rartino
rartino reviewed Mar 10, 2020
View reviewed changes
optimade.rst Outdated Show resolved Hide resolved
@ml-evs ml-evs force-pushed the ml-evs/minor_fix branch from ed8e665 to 9c902d9 Compare March 11, 2020 15:10
@ml-evs ml-evs marked this pull request as ready for review March 11, 2020 15:11
@ml-evs ml-evs force-pushed the ml-evs/minor_fix branch from 9c902d9 to fbd4223 Compare March 11, 2020 15:11
@ml-evs
Copy link
Member Author

ml-evs commented Mar 11, 2020

I haven't been able to spot any more filters that are optional, but are not tagged as such, so I think this is ready for review. We are now validating this list as of Materials-Consortia/optimade-python-tools#213.

rartino
rartino approved these changes Mar 11, 2020
View reviewed changes
Copy link
Contributor

@rartino rartino 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.

Great work with the validator! It seems sensible to push for including it in the main repository to travis-validate changes to the specification, right?

@ml-evs
Copy link
Member Author

ml-evs commented Mar 11, 2020

Looks good.

Great work with the validator! It seems sensible to push for including it in the main repository to travis-validate changes to the specification, right?

To clarify, we are validating our default server implementation against all the mandatory :filter: examples in the spec (and hopefully soon will be doing the same with optional filters). I'm not sure we should go as far as validating the spec with the same tool, but we could discuss this in the next meeting!

@ml-evs ml-evs requested review from CasperWA and merkys March 11, 2020 15:21
merkys
merkys approved these changes Mar 13, 2020
View reviewed changes
Copy link
Member

@merkys merkys 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!

@ml-evs ml-evs merged commit e92995c into develop Mar 13, 2020
@ml-evs ml-evs deleted the ml-evs/minor_fix branch March 13, 2020 13:12
@ml-evs ml-evs mentioned this pull request Mar 13, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rartino rartino rartino approved these changes

@merkys merkys merkys approved these changes

@CasperWA CasperWA Awaiting requested review from CasperWA

Assignees
No one assigned
Labels
PR/minor-fixes-only
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tag optional filters in a parseable way
3 participants
@ml-evs @rartino @merkys