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

Restore test file format #318

Merged
merged 175 commits into from
Jan 25, 2019
Merged

Conversation

AirQuick
Copy link
Member

@AirQuick AirQuick commented Aug 31, 2018

This pull request derives from #317. So needs to be handled after that.


.xspec files in the repository have experienced unrelated format modifications in these few years.
So it's hard to see what the actual changes are.

This pull request restores the original format of these files:

  • test/generate-tests-utils.xspec
  • test/generate-x-utils.xspec
  • test/generate-xspec-tests.xspec
  • test/unit-expect-xsl.xspec
  • test/xspec-focus-2.xspec
  • test/xspec-rule.xspec
  • test/xspec-variable.xspec
  • test/end-to-end/cases/xspec-focus-1.xspec
  • test/end-to-end/cases/xspec-function.xspec
  • test/end-to-end/cases/xspec-import.xspec
  • test/end-to-end/cases/xspec-imported.xspec
  • test/end-to-end/cases/xspec-pending.xspec
  • tutorial/escape-for-regex.xspec

Now you can clearly see what has actually changed in those files.

…c-rule

# Conflicts:
#	build.xml
#	test/saxon-custom-options/config.xml
#	test/saxon-custom-options/test.xspec
#	test/xspec-bat.cmd
#	test/xspec.bats
…st-file-format

# Conflicts:
#	build.xml
#	test/saxon-custom-options/config.xml
#	test/saxon-custom-options/test.xspec
#	test/xspec-bat.cmd
#	test/xspec.bats
…k/xspec into valid-xspec-rule

# Conflicts:
#	test/xspec-bat.cmd
#	test/xspec.bats
…c-rule

# Conflicts:
#	build.xml
#	test/end-to-end/generate-expected.cmd
#	test/end-to-end/generate-expected.sh
#	test/end-to-end/processor/html/_normalizer.xsl
#	test/end-to-end/run-e2e-tests.cmd
#	test/end-to-end/run-e2e-tests.sh
#	test/run-xspec-tests-ant.cmd
#	test/run-xspec-tests-ant.sh
…st-file-format

# Conflicts:
#	build.xml
#	test/end-to-end/generate-expected.cmd
#	test/end-to-end/generate-expected.sh
#	test/end-to-end/processor/html/_normalizer.xsl
#	test/end-to-end/run-e2e-tests.cmd
#	test/end-to-end/run-e2e-tests.sh
#	test/run-xspec-tests-ant.cmd
#	test/run-xspec-tests-ant.sh
…c-rule

# Conflicts:
#	test/xspec-bat.cmd
#	test/xspec.bats
…st-file-format

# Conflicts:
#	test/xspec-bat.cmd
#	test/xspec.bats
…c-rule

# Conflicts:
#	test/xspec-bat.cmd
#	test/xspec.bats
…into restore-test-file-format

# Conflicts:
#	test/xspec.bats
@AirQuick AirQuick added this to the v1.2.0 milestone Dec 24, 2018
…st-file-format

# Conflicts:
#	.travis.yml
#	appveyor.yml
#	test/run-bats.cmd
#	test/run-bats.sh
#	test/win-bats/collection.xml
#	test/xspec.bats
…st-file-format

# Conflicts:
#	test/end-to-end/cases/xspec-rule.xspec
@AirQuick AirQuick added the test label Jan 20, 2019
…st-file-format

# Conflicts:
#	test/generate-tests-utils.xspec
#	test/generate-x-utils.xspec
@AirQuick
Copy link
Member Author

The latest automated tests were not submitted successfully. (Maybe I pushed the last commit too quickly.)
They are successful on my repository: Travis, AppVeyor

@cirulls
Copy link
Member

cirulls commented Jan 24, 2019

@AirQuick: I tried to trace the failed build in AppVeyor or to trigger manually the webhook in GitHub but without success. I wonder if you could push a dummy commit to the branch of this pull request to re-trigger the test build on AppVeyor, this should turn the checks into green. Since we are always doing squash and merge into master the dummy commit should not add junk in your branch.

If you have a better idea to turn the AppVeyor check into green, feel free to share it.

Ignore it
This reverts commit 7ee46fa.
@AirQuick
Copy link
Member Author

Thanks @cirulls , your idea has made this pr green again.

@AirQuick AirQuick merged commit 7dadb6e into xspec:master Jan 25, 2019
@AirQuick AirQuick deleted the restore-test-file-format branch January 25, 2019 12:41
@AirQuick
Copy link
Member Author

@cirulls
Now merged.

@galtm
#475 now conflicts with the master.
As described in the initial comment, this change pertains only to format. Nothing in xspec-variable.xspec has been changed in essence.
Hope you can resolve the conflict easily honoring the restored format.

@galtm
Copy link
Member

galtm commented Jan 25, 2019

I should have some time today and/or over the weekend to catch up on your recent comments.

@cirulls
Copy link
Member

cirulls commented Jan 28, 2019

@AirQuick: thanks, I added this to the release notes.

I think I am party responsible of weird formatting on some files. Just out of interest and for future commits: do you use the automatic format and indent in oXygen to format files before committing them? Or do you have any rules for formatting files (e.g. tabs, number of spaces, etc.)?

@AirQuick
Copy link
Member Author

Formatting wasn't weird and I think it's fine to change it occasionally (inserting some spaces, wrapping too long grew lines).
The problem was that formatting changes were intermingled with actual changes, which made it difficult to see the actual history from 5105319. I was so puzzled that I had to restore the original format before proposing further changes.

I usually use the automatic formatting on oXygen and do so when I make initial commits (because this repository hasn't established any rule yet) or modify a file which was first created by me (because I know its initial automatic formatting rule) or refactor a file broadly (because the changes are disruptive anyway).
When I just modify an existing file which was first submitted by other people, I refrain from the automatic formatting because doing so makes it difficult to track actual changes.

At some point in future we may want to

  1. establish our formatting rule based on oXygen's automatic formatting options (just because oXygen appears to be the most popular)
  2. create an oXygen project file (xspec.xpr?) and put it in the repository root (and beautify it? - I have no experience with it)
  3. set the project level options to realize the automatic formatting rule
  4. enforce the automatic formatting on new files submitted to this repository
  5. apply the automatic formatting to some of the existing files when they are considered very stable

@cirulls
Copy link
Member

cirulls commented Feb 9, 2019

This sounds like a good plan although it may take some time to implement it and enforce it via the CI systems. I created #515 to record this conversation in an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants