Skip to content

Conversation

@DiscJockeyDJ
Copy link
Contributor

No description provided.

Copy link
Collaborator

@rossmills99 rossmills99 left a comment

Choose a reason for hiding this comment

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

It would be good to see a test introduced to verify the TransactionId HeadDocumentHeader value is handled properly.

Can we move the x-arango-trx-id to a constant?

Moved the TransactionId header string to a constant class.

fix ArangoDB-Community#322
@DiscJockeyDJ
Copy link
Contributor Author

PR updated to address the comments,

test introduced to verify the TransactionId HeadDocumentHeader value is handled properly.

a constant for x-arango-trx-id?

Copy link
Collaborator

@rossmills99 rossmills99 left a comment

Choose a reason for hiding this comment

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

I think the changes look good! As discussed elsewhere, we need to annotate the new tests with [Trait("Feature", "StreamTransaction")] so they are skipped in the 3.4 ArangoDB test run.

@DiscJockeyDJ
Copy link
Contributor Author

I think the changes look good! As discussed elsewhere, we need to annotate the new tests with [Trait("Feature", "StreamTransaction")] so they are skipped in the 3.4 ArangoDB test run.

PR updated

@rossmills99 rossmills99 merged commit 9d1ac5e into ArangoDB-Community:master Sep 15, 2021
PostDocumentsQuery query = null,
ApiClientSerializationOptions serializationOptions = null);
ApiClientSerializationOptions serializationOptions = null,
HeadDocumentHeader headers = null);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor thing: we could rename HeadDocumentHeader to DocumentHeader or DocumentHeaderProperties to be in line with CursorHeaderProperties.

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.

3 participants