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

Remove batchSequenceNumber #18

Merged

Conversation

promag
Copy link
Contributor

@promag promag commented Sep 22, 2016

With this PR the file generation can be idempotent, ie, it's is possible to create several File objects with the same setup and each will have the same content. This is most relevant in tests .

@@ -73,12 +71,9 @@ File.prototype._validate = function() {
File.prototype.addBatch = function(batch) {

// Set the batch number on the header and control records
batch.header.batchNumber.value = batchSequenceNumber
batch.control.batchNumber.value = batchSequenceNumber
batch.header.batchNumber.value = this._batches.length
Copy link
Collaborator

Choose a reason for hiding this comment

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

Although this generates a sequential batch number, it could be useful to have a batchSequenceNumber that could be initialized when creating the File, instead of starting always with 0.

WDYGT?

@promag promag force-pushed the remove-batch-sequence-number branch 2 times, most recently from de4ed02 to c959f28 Compare September 29, 2016 21:17

// Validate all values

this._batchSequenceNumber = Number(options.batchSequenceNumber) || 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing ;

@joaopaulofonseca joaopaulofonseca merged commit 141ae63 into glenselle:master Sep 29, 2016
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.

2 participants