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

Cleanup tests #16

Merged
merged 9 commits into from
Jun 17, 2018
Merged

Cleanup tests #16

merged 9 commits into from
Jun 17, 2018

Conversation

raphinesse
Copy link
Contributor

@raphinesse raphinesse commented Jun 3, 2018

This builds on top and further improves upon the great test cleanup done by @brodybits in #14. Due to #8 being blocked for now, I removed all related changes in the hopes that we can merge the remaining changes ASAP.

If you want to compare this with the original changes by @brodybits, compare against the second commit
5f0d79e. Everything after that is new.

I left the commits separate so the whole thing is easier to review. We can squash all of this on merge.

raphinesse and others added 9 commits June 3, 2018 21:28
Diff best viewed with --anchored=checkConfigXml

Co-authored-by: Christopher J. Brody <brodybits@litehelpers.net>
Co-authored-by: Raphael von der Grün <raphinesse@gmail.com>
Co-authored-by: Christopher J. Brody <brodybits@litehelpers.net>
Co-authored-by: Raphael von der Grün <raphinesse@gmail.com>
@brodybits
Copy link

Seems to be missing check for presence vs absence of .gitignore and .npmignore that I had proposed in f4b752d. I think this is really needed for the following reasons:

  • There are some cases where cordova-create generates unwanted .npmignore file, due to what happens when cordova-create uses cordova-app-hello-world via NPM. I think we want the actual behavior to be known today and verified fixed for good in the future.
  • I also would like to see the actual behavior related to the .gitignore file to be known today, verified to be what we want in the future.

Otherwise looks like very nice work.

@brodybits
Copy link

Seems to be missing check for presence vs absence of .gitignore and .npmignore that I had proposed in f4b752d.

I would be fine to leave this part for another PR, if we can do this today.

Agreed to squash these in the merge.

@raphinesse
Copy link
Contributor Author

@stevengill @dpogue @shazron If no one objects, I'm going to squash-merge this as soon as I find time for another self-review.

Copy link
Member

@dpogue dpogue left a comment

Choose a reason for hiding this comment

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

👍 from me

@raphinesse raphinesse merged commit 3d21f05 into apache:master Jun 17, 2018
@raphinesse raphinesse deleted the cleanup-tests branch June 17, 2018 12:28
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.

None yet

3 participants