Skip to content

Conversation

@evanweible-wf
Copy link
Contributor

Issue

#45

Changes

Source:

  • Delete the entire collections/ directory that holds the temporary collection files after they have been read and merged.

Tests:

  • n/a

Areas of Regression

  • coverage

Testing

  • on master:
    • ddev coverage --integration
    • verify that temp file in coverage/collections/ was not deleted
  • on this branch
    • ddev coverage --integration
    • verify that temp directory coverage/collections/ was deleted entirely

Code Review

@trentgrover-wf
@maxwellpeterson-wf
@dustinlessard-wf
@jayudey-wf

@evanweible-wf evanweible-wf added this to the 1.0.1 milestone Sep 3, 2015
@maxwellpeterson-wf
Copy link
Member

+1

@codecov-io
Copy link

Current coverage is 48.05%

Merging #71 into master will not affect coverage as of 1406afa

Powered by Codecov. Updated on successful CI builds.

@dustinlessard-wf
Copy link

+1

@jayudey-wf jayudey-wf changed the title Delete first temporary coverage file. CP-962 Delete first temporary coverage file. Sep 3, 2015
@trentgrover-wf
Copy link
Contributor

+1
@jayudey-wf ready for merge

@evanweible-wf evanweible-wf force-pushed the delete-first-coverage-file branch from e5b5a19 to 1272dd3 Compare September 3, 2015 21:23
@evanweible-wf
Copy link
Contributor Author

@trentgrover-wf @maxwellpeterson-wf @dustinlessard-wf

At @jayudey-wf's suggestion, I'm now deleting the collections/ directory that would contain all of these temporary files instead of deleting each individual file. Should be faster, too.

@evanweible-wf evanweible-wf force-pushed the delete-first-coverage-file branch from 1272dd3 to 06f7f08 Compare September 3, 2015 21:38
@trentgrover-wf
Copy link
Contributor

+1
good catch @jayudey-wf

@evanweible-wf
Copy link
Contributor Author

@maxwellpeterson-wf
Copy link
Member

+1

@evanweible-wf
Copy link
Contributor Author

@jayudey-wf ready for merge

@jayudey-wf
Copy link
Contributor

QA Resource Approval: +10

  • Testing instruction
  • Dev +1's
  • Dev/QA +10 with detail of what was tested
    • installed branch into a dart project and verified that collection directory was being appropriately deleted
  • Unit test created/updated
  • All unit tests pass

Merging into master.

jayudey-wf added a commit that referenced this pull request Sep 8, 2015
CP-962 Delete first temporary coverage file.
@jayudey-wf jayudey-wf merged commit efc215e into master Sep 8, 2015
@evanweible-wf evanweible-wf deleted the delete-first-coverage-file branch November 24, 2015 17:05
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.

7 participants