Skip to content

Switch to LuLPeg, update readme and rockspec #4

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

Merged
merged 1 commit into from
Dec 16, 2019

Conversation

RunsFor
Copy link

@RunsFor RunsFor commented Dec 12, 2019

We decided to switch to lulpeg for now, since it is written in lua and appears to be more stable. Unlike lpeg, lulpeg is available in tarantool ecosystem.

Anyway, we may return to this later, when performance issues pops up.

Tests:

tarantool tests/runner.lua gives this:

IMAGE 2019-12-12 16:29:28

@olegrok olegrok self-requested a review December 12, 2019 13:32
@RunsFor RunsFor requested a review from Totktonada December 15, 2019 15:06
@Totktonada
Copy link
Member

We decided to switch to lulpeg for now, since it is written in lua and appears to be more stable. Unlike lpeg, lulpeg is available in tarantool ecosystem.

It is great that the motivation of the change is explicit now, but I would like to see it right in the commit message: this way I'll able to see why things were changed w/o jumps between git log and GitHub PRs.

I would also make it explicit that lpeg segfaults in some environments rather then vague 'less stable'.

Please, update the commit message and proceed (or just proceed if you're disagree with me).

While we're here: will we create a merge commit for each merged PR (Merge pull request button) or will keep master's history flat (Rebase and merge button)? Let's decide and keep it consistent.

Personally I prefer flat history, but let's decide based on preferences of ones who want to contribute here.

Copy link
Member

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

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

LGTM.

@Totktonada
Copy link
Member

BTW, I suggest to close #3 from the corresponsing commit message, because in fact your commit fixes the issue.

We decided to switch to `lulpeg` for now.
It is written in lua and appears to be more stable then lpeg,
which happen to crash under tarantool (or luajit). Additional
research on this topic is needed.

Also, unlike lpeg, lulpeg is available in tarantool ecosystem.

We may return to the topic lpeg/lulpeg later, when performance issues
pops up.

Removed graphql/package.lua, since there are no plans to distribute
package through lit right now.
@RunsFor RunsFor force-pushed the update-readme-and-rockspec branch from bf8708b to 82e3af5 Compare December 16, 2019 15:16
@RunsFor
Copy link
Author

RunsFor commented Dec 16, 2019

will we create a merge commit for each merged PR (Merge pull request button) or will keep master's history flat (Rebase and merge button)? Let's decide and keep it consistent.

There is also a "Squash and merge" button. What about that? Docs says it uses fast-forward merge. Lets try to keep it flat, but not require it to be flat.

@RunsFor RunsFor merged commit c11b107 into master Dec 16, 2019
@RunsFor RunsFor deleted the update-readme-and-rockspec branch December 16, 2019 15:37
@Totktonada
Copy link
Member

Squash and merge button is what that should never be used, IMHO. Commits should be atomic.

I know, it is possible to keep short-term branches (ones where a developer working on a bug or a feature) full of intermediate changes and them squash them at once. The only reason to do so it to refraining from force pushes. However it is completely okay to force-push a short-term branch. So there are no pros in this process, only cons: unability to review changes in its final state.

Aside of this, considering commits atomicity, if you squash commits before push, then you are unable to propose a patchset with more then one commit within one PR.

I strongly against squashing commits before push. This approach leads to a mess in many ways and this just should not be used. Or maybe should be used only for toy projects, where no review process exists and vcs history readability is not matter at all.

DifferentialOrange pushed a commit that referenced this pull request Nov 17, 2021
GraphQL custom directives was introduced in spec #3.13 in October 2021
release [1]. Directives are the preferred way to extend GraphQL with
custom or experimental behavior. This patch adds support of custom
directives, as well as several location adjustments and repeatable
directive option needed for full support of custom directives.

This patch also adds introspection test for schema with custom
directives to validate schema. Introspection of schema is described in
spec #4.2 .

1. https://spec.graphql.org/October2021/#sec-Type-System.Directives.Custom-Directives

Based on PR #20 by @no1seman
DifferentialOrange pushed a commit that referenced this pull request Nov 17, 2021
GraphQL custom directives was introduced in spec #3.13 in October 2021
release [1]. Directives are the preferred way to extend GraphQL with
custom or experimental behavior. This patch adds support of custom
directives, as well as several location adjustments and repeatable
directive option needed for full support of custom directives.

This patch also adds introspection test for schema with custom
directives to validate schema. Introspection of schema is described in
spec #4.2 [2].

1. https://spec.graphql.org/October2021/#sec-Type-System.Directives.Custom-Directives
2. https://spec.graphql.org/October2021/#sec-Schema-Introspection

Based on PR #20 by @no1seman
DifferentialOrange pushed a commit that referenced this pull request Nov 17, 2021
GraphQL custom directives was introduced in spec #3.13 in October 2021
release [1]. Directives are the preferred way to extend GraphQL with
custom or experimental behavior. This patch adds support of custom
directives, as well as several location adjustments and repeatable
directive option needed for full support of custom directives.

This patch also adds introspection test for schema with custom
directives to validate schema. Introspection of schema is described in
spec #4.2 [2].

1. https://spec.graphql.org/October2021/#sec-Type-System.Directives.Custom-Directives
2. https://spec.graphql.org/October2021/#sec-Schema-Introspection

Based on PR #20 by @no1seman
DifferentialOrange pushed a commit that referenced this pull request Nov 18, 2021
GraphQL custom directives was introduced in spec #3.13 in October 2021
release [1]. Directives are the preferred way to extend GraphQL with
custom or experimental behavior. This patch adds support of custom
directives, as well as several location adjustments and repeatable
directive option needed for full support of custom directives.

This patch also adds introspection test for schema with custom
directives to validate schema. Introspection of schema is described in
spec #4.2 [2].

1. https://spec.graphql.org/October2021/#sec-Type-System.Directives.Custom-Directives
2. https://spec.graphql.org/October2021/#sec-Schema-Introspection

Based on PR #20 by @no1seman
DifferentialOrange pushed a commit that referenced this pull request Nov 18, 2021
GraphQL custom directives was introduced in spec #3.13 in October 2021
release [1]. Directives are the preferred way to extend GraphQL with
custom or experimental behavior. This patch adds support of custom
directives, as well as several location adjustments and repeatable
directive option needed for full support of custom directives.

This patch also adds introspection test for schema with custom
directives to validate schema. Introspection of schema is described in
spec #4.2 [2].

1. https://spec.graphql.org/October2021/#sec-Type-System.Directives.Custom-Directives
2. https://spec.graphql.org/October2021/#sec-Schema-Introspection

Based on PR #20 by @no1seman
DifferentialOrange pushed a commit that referenced this pull request Nov 18, 2021
GraphQL custom directives was introduced in spec #3.13 in October 2021
release [1]. Directives are the preferred way to extend GraphQL with
custom or experimental behavior. This patch adds support of custom
directives, as well as several location adjustments and repeatable
directive option needed for full support of custom directives.

This patch also adds introspection test for schema with custom
directives to validate schema. Introspection of schema is described in
spec #4.2 [2].

1. https://spec.graphql.org/October2021/#sec-Type-System.Directives.Custom-Directives
2. https://spec.graphql.org/October2021/#sec-Schema-Introspection

Based on PR #20 by @no1seman
DifferentialOrange pushed a commit that referenced this pull request Nov 18, 2021
GraphQL custom directives was introduced in spec #3.13 in October 2021
release [1]. Directives are the preferred way to extend GraphQL with
custom or experimental behavior. This patch adds support of custom
directives, as well as several location adjustments and repeatable
directive option needed for full support of custom directives.

This patch also adds introspection test for schema with custom
directives to validate schema. Introspection of schema is described in
spec #4.2 [2].

1. https://spec.graphql.org/October2021/#sec-Type-System.Directives.Custom-Directives
2. https://spec.graphql.org/October2021/#sec-Schema-Introspection

Based on PR #20 by @no1seman
DifferentialOrange pushed a commit that referenced this pull request Nov 18, 2021
GraphQL custom directives was introduced in spec #3.13 in October 2021
release [1]. Directives are the preferred way to extend GraphQL with
custom or experimental behavior. This patch adds support of custom
directives, as well as several location adjustments and repeatable
directive option needed for full support of custom directives.

This patch also adds introspection test for schema with custom
directives to validate schema. Introspection of schema is described in
spec #4.2 [2].

1. https://spec.graphql.org/October2021/#sec-Type-System.Directives.Custom-Directives
2. https://spec.graphql.org/October2021/#sec-Schema-Introspection

Based on PR #20 by @no1seman
DifferentialOrange pushed a commit that referenced this pull request Nov 19, 2021
GraphQL custom directives was introduced in spec #3.13 in October 2021
release [1]. Directives are the preferred way to extend GraphQL with
custom or experimental behavior. This patch adds support of custom
directives, as well as several location adjustments and repeatable
directive option needed for full support of custom directives.

This patch also adds introspection test for schema with custom
directives to validate schema. Introspection of schema is described in
spec #4.2 [2].

1. https://spec.graphql.org/October2021/#sec-Type-System.Directives.Custom-Directives
2. https://spec.graphql.org/October2021/#sec-Schema-Introspection

Based on PR #20 by @no1seman
DifferentialOrange pushed a commit that referenced this pull request Nov 23, 2021
GraphQL custom directives was introduced in spec #3.13 in October 2021
release [1]. Directives are the preferred way to extend GraphQL with
custom or experimental behavior. This patch adds support of custom
directives, as well as several location adjustments and repeatable
directive option needed for full support of custom directives.

This patch also adds introspection test for schema with custom
directives to validate schema. Introspection of schema is described in
spec #4.2 [2].

1. https://spec.graphql.org/October2021/#sec-Type-System.Directives.Custom-Directives
2. https://spec.graphql.org/October2021/#sec-Schema-Introspection

Based on PR #20 by @no1seman
DifferentialOrange pushed a commit that referenced this pull request Nov 23, 2021
GraphQL custom directives was introduced in spec #3.13 in October 2021
release [1]. Directives are the preferred way to extend GraphQL with
custom or experimental behavior. This patch adds support of custom
directives, as well as several location adjustments and repeatable
directive option needed for full support of custom directives.

This patch also adds introspection test for schema with custom
directives to validate schema. Introspection of schema is described in
spec #4.2 [2].

1. https://spec.graphql.org/October2021/#sec-Type-System.Directives.Custom-Directives
2. https://spec.graphql.org/October2021/#sec-Schema-Introspection

Based on PR #20 by @no1seman
Totktonada pushed a commit that referenced this pull request Nov 30, 2021
GraphQL custom directives was introduced in spec #3.13 in October 2021
release [1]. Directives are the preferred way to extend GraphQL with
custom or experimental behavior. This patch adds support of custom
directives, as well as several location adjustments and repeatable
directive option needed for full support of custom directives.

This patch also adds introspection test for schema with custom
directives to validate schema. Introspection of schema is described in
spec #4.2 [2].

1. https://spec.graphql.org/October2021/#sec-Type-System.Directives.Custom-Directives
2. https://spec.graphql.org/October2021/#sec-Schema-Introspection

Based on PR #20 by @no1seman
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.

4 participants