Spelling #802

2017-09-04T04:01:33Z

jsoref commented

2017-09-04 04:01:33 +00:00

(Migrated from github.com)

You can of course drop commits.
The filename change is a bit odd -> SheetJS/jxls#2 (someone would have to fix this repo once that PR is taken, if it is).

The password change is because it's an annoying false positive. afaict, there's no particular requirement to misspell the word by omitting a character, and thus it's nicer for spelling tools if you just append a suffix which should also result in a non-matching password. There's also a family of tokens such as C4tegory.

You can of course drop commits. The filename change is a bit odd -> SheetJS/jxls#2 (someone would have to fix this repo once that PR is taken, if it is). The password change is because it's an annoying false positive. afaict, there's no particular requirement to misspell the word by omitting a character, and thus it's nicer for spelling tools if you just append a suffix which should also result in a non-matching password. There's also a family of tokens such as `C4tegory`.

👍 1 ❤️ 1 🎉 1

SheetJSDev commented

2017-09-04 04:31:51 +00:00

(Migrated from github.com)

Thanks for looking into this!

The dynamic error corresponds to a test file with that name (it's pulled from the jXLS project test suite ), so that should be preserved.

Instead of passwor_, change the password to passwords or Password or PASSWORD (pick your favorite).

Other weird names like C4tegory appear in the test script as fake names for the various properties. The pertinent test starts from the JS representation, writes files, reads the new files, and verifies that the properties of the new file match the original representation. The object's keys should not be changed, but the values can be changed. Feel free to pick your favorite sesquipedalians!

Please undo the commit corresponding to dynamic, change passwor, and squash down to one commit with the message spelling [ci skip] (the [ci skip] part will skip travis tests)

Thanks for looking into this! The `dynamic` error corresponds to a test file with that name (it's pulled from the [jXLS project test suite](https://github.com/SheetJS/jxls/blob/master/jxls-examples/src/main/resources/templates/dynamicolumns.xls) ), so that should be preserved. Instead of `passwor_`, change the password to `passwords` or `Password` or `PASSWORD` (pick your favorite). Other weird names like `C4tegory` appear in the [test script](https://github.com/SheetJS/js-xlsx/blob/master/test.js#L1255) as fake names for the various properties. The pertinent test starts from the JS representation, writes files, reads the new files, and verifies that the properties of the new file match the original representation. The [object's keys](https://github.com/SheetJS/js-xlsx/blob/master/test.js#L1255-L1269) should not be changed, but the values can be changed. Feel free to pick your favorite sesquipedalians! Please undo the commit corresponding to `dynamic`, change `passwor`, and squash down to one commit with the message `spelling [ci skip]` (the `[ci skip]` part will skip travis tests)

SheetJSDev commented

2017-09-04 06:16:01 +00:00

(Migrated from github.com)

The other changes in #803 look good, if you can fold those changes from #803 into this PR with one commit, we can merge as-is! (normally we would do some git jujutsu but GH doesn't recognize a commit as a successful merge unless it has the same exact hash :/

Make the message for the combined commit:

spelling [ci skip]

- fixed spelling errors in README and code (fixes #802)
- replaced garbled names in property tests (fixes #803)

(GH will automatically add a note in both PRs linking to the commit)

The other changes in #803 look good, if you can fold those changes from #803 into this PR with one commit, we can merge as-is! (normally we would do some git jujutsu but GH doesn't recognize a commit as a successful merge unless it has the same exact hash :/ Make the message for the combined commit: ```js spelling [ci skip] - fixed spelling errors in README and code (fixes #802) - replaced garbled names in property tests (fixes #803) ``` (GH will automatically add a note in both PRs linking to the commit)

Sign in to join this conversation.

No reviewers