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.

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

Fixes #436 Properties in RunningAqlQuery class should be PascalCase instead of lowerCase #438

Merged

Conversation

JLedel
Copy link
Contributor

@JLedel JLedel commented Oct 26, 2022

No description provided.

@JLedel
Copy link
Contributor Author

JLedel commented Oct 27, 2022

Currently fails due to ba458b7, but should be unaffected so will probably solve itself once that problem is sorted out.

@DiscoPYF
Copy link
Collaborator

DiscoPYF commented Nov 1, 2022

Thanks @JLedel . This is technically a breaking change for consumers so I wonder if we should hold off merging it to master for now. We plan to create an intermediary 1.3.0 release with a bunch of enhancements, if so we don't want to introduce any breaking changes.

We want to group all breaking changes together in a major release whenever possible.

Or maybe it's minor enough that we should just get it in? The more we wait, the more people will use it 🤔

@tjoubert What do you think?

For now, I've added the breaking-change label for clarity.

@DiscoPYF DiscoPYF added the breaking change Identify issues that contain a breaking change for existing consumer code label Nov 1, 2022
@DiscoPYF
Copy link
Collaborator

DiscoPYF commented Nov 1, 2022

Currently fails due to ba458b7, but should be unaffected so will probably solve itself once that problem is sorted out.

Can you rebase your branch on master and push? So that we can see that the changes pass the CI checks.

@tjoubert
Copy link
Contributor

tjoubert commented Nov 1, 2022

Thanks @JLedel . This is technically a breaking change for consumers so I wonder if we should hold off merging it to master for now. We plan to create an intermediary 1.3.0 release with a bunch of enhancements, if so we don't want to introduce any breaking changes.

We want to group all breaking changes together in a major release whenever possible.

Or maybe it's minor enough that we should just get it in? The more we wait, the more people will use it 🤔

@tjoubert What do you think?

For now, I've added the breaking-change label for clarity.

@DiscoPYF , I agree. Let's group this along with the other breaking changes for a major release after 1.3.0. @JLedel , thank you for the contribution.

@JLedel JLedel force-pushed the 436-running-aql-query-pascal-case branch from 8966e6b to 3a23d08 Compare November 1, 2022 13:16
@JLedel
Copy link
Contributor Author

JLedel commented Nov 1, 2022

Currently fails due to ba458b7, but should be unaffected so will probably solve itself once that problem is sorted out.

Can you rebase your branch on master and push? So that we can see that the changes pass the CI checks.

Check and check!

@DiscoPYF DiscoPYF merged commit 961e89d into ArangoDB-Community:master Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Identify issues that contain a breaking change for existing consumer code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Properties in RunningAqlQuery class should be PascalCase instead of lowerCase
3 participants