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.

Sign up for GitHub

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

Jump to bottom

diff-lcs has changed its format in 1.4 #421

Merged
merged 4 commits into from Jun 29, 2020
Merged

diff-lcs has changed its format in 1.4 #421

merged 4 commits into from Jun 29, 2020

Conversation

JonRowe
Copy link
Member

@JonRowe JonRowe commented Jun 27, 2020

The recent diff-lcs update changed some parts of the diff output, breaking our builds, these seem trivial enough to ignore (e.g. they don't seem wrong, just different).

Draft because I'm going to improve the formatting of this spec a bit whilst I'm here, and I'm going to setup CI to test both versions of diff-lcs.

@JonRowe
In version 1.4 diff-lcs has minorly changed its format
357cc8b 
This commit deals with both versions, as its just header notation thats
changed. I'm not sure either is more correct than the other but its
harmless to our diffing process.

Some additional `\n` changes are due to the string type change, `"\\n"`
is the same as `'\n'`.
@JonRowe JonRowe force-pushed the fix-differ-support branch 5 times, most recently from f7e2912 to de1b664 Compare June 28, 2020 10:11
@JonRowe JonRowe mentioned this pull request Jun 28, 2020
Merged
@JonRowe
Copy link
Member Author

JonRowe commented Jun 28, 2020 •
edited

Related I've asked the diff-lcs team to bump their major version if they make Ruby changes in halostatue/diff-lcs#62 they have dropped "soft" support for older rubies, which in turn means 1.4 is broken on our older Rubies.

@JonRowe JonRowe force-pushed the fix-differ-support branch 2 times, most recently from 9ee1b82 to 68a10d1 Compare June 28, 2020 10:47
@JonRowe JonRowe mentioned this pull request Jun 28, 2020
Merged
@JonRowe JonRowe marked this pull request as ready for review June 28, 2020 10:51
@JonRowe JonRowe requested a review from benoittgt June 28, 2020 10:51
@halostatue
Copy link

Can you detail the changes that were breaking for you on this with examples as a bug against diff-lcs? As I mentioned in halostatue/diff-lcs#62, I am willing to consider some changes to how diff-lcs works when running under RSpec so that we keep compatibility as much as possible.

@JonRowe
Copy link
Member Author

JonRowe commented Jun 28, 2020

@halostatue This PR contains examples of the changed output, the line number headers have changed in some cases, and some unchanged lines have been removed, I'm totally fine with working around that in the two versions like this PR does :)

@halostatue halostatue mentioned this pull request Jun 28, 2020
Merged
benoittgt
benoittgt reviewed Jun 29, 2020
View reviewed changes
Copy link
Member

@benoittgt benoittgt left a comment

Choose a reason for hiding this comment

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

Thanks for all the changes in differ_spec. It is much better like this.

Gemfile Outdated Show resolved Hide resolved
spec/rspec/support/differ_spec.rb
|
|@@ #{one_line_header} @@
|-Tu avec carte {count} item has
|+Tu avec carté {count} itém has
Copy link
Member

Choose a reason for hiding this comment

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

Lol this means nothing in french. 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Nothing like a good bit of franglais, its only the é thats important if you'd like to make it make sense :P

@JonRowe JonRowe force-pushed the fix-differ-support branch 2 times, most recently from 1a3e673 to ca6729a Compare June 29, 2020 14:16
@JonRowe JonRowe merged commit 64a14e2 into master Jun 29, 2020
@JonRowe JonRowe deleted the fix-differ-support branch June 29, 2020 14:23
JonRowe added a commit that referenced this pull request Jun 29, 2020
@JonRowe
Merge pull request #421 from rspec/fix-differ-support
9cceff0 
`diff-lcs` has changed its format in 1.4
@JonRowe JonRowe mentioned this pull request Jul 1, 2020
Closed
@mvz mvz mentioned this pull request Aug 21, 2020
Merged
benoittgt added a commit to rspec/rspec-dev that referenced this pull request Sep 27, 2020
@benoittgt
diff-lcs has changed its format in 1.4
d97105d 
See: rspec/rspec-support#421
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this pull request Oct 6, 2021
@JonRowe
[support] Merge pull request rspec/rspec-support#421 from rspec/fix-d…
6752b8a 
…iffer-support

`diff-lcs` has changed its format in 1.4

---
This commit was imported from rspec/rspec-support@9cceff0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@halostatue halostatue halostatue left review comments

@benoittgt benoittgt benoittgt left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@JonRowe @halostatue @benoittgt