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

feat: Add deep diffs to PRs #366

Merged
merged 14 commits into from
Aug 4, 2021
Merged

feat: Add deep diffs to PRs #366

merged 14 commits into from
Aug 4, 2021

Conversation

guseggert
Copy link
Contributor

@guseggert guseggert commented Jul 20, 2021

  • add scripts/diff.js which finds the changed objects and then diffs
    them all
  • on a pull request, posts the diff as a comment on the PR

- add scripts/diff.js which finds the changed objects and then diffs
them all
- on a pull request, posts the diff as a comment on the PR
@guseggert guseggert requested a review from aschmahmann July 20, 2021 14:23
@github-actions
Copy link

Diff of changes:

diff --git a/tmp/tmp.y37DTdWWZR/index.xml b/tmp/tmp.2iIKF8VATO/index.xml
index 56fd02f..bdc9ac6 100644
--- a/tmp/tmp.y37DTdWWZR/index.xml
+++ b/tmp/tmp.2iIKF8VATO/index.xml
@@ -6,7 +6,7 @@
 <description>Recent releases on IPFS Distributions</description>
 <generator>Hugo -- gohugo.io</generator>
 <language>en-us</language>
- <lastBuildDate>Fri, 09 Jul 2021 00:57:29 +0200</lastBuildDate>
+ <lastBuildDate>Tue, 20 Jul 2021 14:34:34 +0000</lastBuildDate>
 
 <atom:link href="https://dist.ipfs.io/index.xml" rel="self" type="application/rss&#43;xml" />
 
@@ -15,7 +15,7 @@
 <item>
 <title>fs-repo-migrations</title>
 <link>https://dist.ipfs.io/#fs-repo-migrations</link>
- <pubDate>Tue, 30 Mar 2021 22:00:00 GMT</pubDate>
+ <pubDate>Wed, 31 Mar 2021 00:00:00 GMT</pubDate>
 <description>fs-repo-migrations is a tool for migrating IPFS storage repositories to newer versions.</description>
 </item>
 
@@ -23,7 +23,7 @@
 <item>
 <title>go-ipfs</title>
 <link>https://dist.ipfs.io/#go-ipfs</link>
- <pubDate>Tue, 22 Jun 2021 22:00:00 GMT</pubDate>
+ <pubDate>Wed, 23 Jun 2021 00:00:00 GMT</pubDate>
 <description><p>go-ipfs is the main implementation of IPFS. It includes:</p>
 <ul>
 <li>an IPFS core implementation</li>
@@ -39,7 +39,7 @@
 <item>
 <title>gx</title>
 <link>https://dist.ipfs.io/#gx</link>
- <pubDate>Wed, 06 Mar 2019 23:00:00 GMT</pubDate>
+ <pubDate>Thu, 07 Mar 2019 00:00:00 GMT</pubDate>
 <description>gx is a package management tool built on ipfs</description>
 </item>
 
@@ -47,7 +47,7 @@
 <item>
 <title>gx-go</title>
 <link>https://dist.ipfs.io/#gx-go</link>
- <pubDate>Sun, 07 Oct 2018 22:00:00 GMT</pubDate>
+ <pubDate>Mon, 08 Oct 2018 00:00:00 GMT</pubDate>
 <description>gx-go is a gx helper tool for golang</description>
 </item>
 
@@ -55,7 +55,7 @@
 <item>
 <title>ipfs-cluster-ctl</title>
 <link>https://dist.ipfs.io/#ipfs-cluster</link>
- <pubDate>Wed, 07 Jul 2021 22:00:00 GMT</pubDate>
+ <pubDate>Thu, 08 Jul 2021 00:00:00 GMT</pubDate>
 <description>IPFS Cluster allows to allocate, replicate and track Pins across a cluster of IPFS daemons. ipfs-cluster-ctl is the command-line interface to manage a Cluster peer (run by ipfs-cluster-service).</description>
 </item>
 
@@ -63,7 +63,7 @@
 <item>
 <title>ipfs-cluster-follow</title>
 <link>https://dist.ipfs.io/#ipfs-cluster</link>
- <pubDate>Wed, 07 Jul 2021 22:00:00 GMT</pubDate>
+ <pubDate>Thu, 08 Jul 2021 00:00:00 GMT</pubDate>
 <description>ipfs-cluster-follow runs an IPFS Cluster follower peer, allowing users to easily join and be part of collaborative clusters.</description>
 </item>
 
@@ -71,7 +71,7 @@
 <item>
 <title>ipfs-cluster-service</title>
 <link>https://dist.ipfs.io/#ipfs-cluster</link>
- <pubDate>Wed, 07 Jul 2021 22:00:00 GMT</pubDate>
+ <pubDate>Thu, 08 Jul 2021 00:00:00 GMT</pubDate>
 <description>IPFS Cluster allows to allocate, replicate and track Pins across a cluster of IPFS daemons. ipfs-cluster-service runs a full Cluster peer.</description>
 </item>
 
@@ -79,7 +79,7 @@
 <item>
 <title>ipfs-ds-convert</title>
 <link>https://dist.ipfs.io/#ipfs-ds-convert</link>
- <pubDate>Wed, 17 Feb 2021 23:00:00 GMT</pubDate>
+ <pubDate>Thu, 18 Feb 2021 00:00:00 GMT</pubDate>
 <description>ipfs-ds-convert is a tool for migrating data between various IPFS datastore configurations</description>
 </item>
 
@@ -87,7 +87,7 @@
 <item>
 <title>ipfs-pack</title>
 <link>https://dist.ipfs.io/#ipfs-pack</link>
- <pubDate>Sun, 10 Dec 2017 23:00:00 GMT</pubDate>
+ <pubDate>Mon, 11 Dec 2017 00:00:00 GMT</pubDate>
 <description>A filesystem packing tool</description>
 </item>
 
@@ -95,7 +95,7 @@
 <item>
 <title>ipfs-see-all</title>
 <link>https://dist.ipfs.io/#ipfs-see-all</link>
- <pubDate>Mon, 10 Oct 2016 22:00:00 GMT</pubDate>
+ <pubDate>Tue, 11 Oct 2016 00:00:00 GMT</pubDate>
 <description>A diagnostics and recovery tool for ipfs repos.</description>
 </item>
 
@@ -103,7 +103,7 @@
 <item>
 <title>ipfs-update</title>
 <link>https://dist.ipfs.io/#ipfs-update</link>
- <pubDate>Tue, 30 Mar 2021 22:00:00 GMT</pubDate>
+ <pubDate>Wed, 31 Mar 2021 00:00:00 GMT</pubDate>
 <description>ipfs-update is a CLI tool to help update and install IPFS easily.</description>
 </item>
 
@@ -111,7 +111,7 @@
 <item>
 <title>ipget</title>
 <link>https://dist.ipfs.io/#ipget</link>
- <pubDate>Tue, 20 Apr 2021 22:00:00 GMT</pubDate>
+ <pubDate>Wed, 21 Apr 2021 00:00:00 GMT</pubDate>
 <description>wget for IPFS: retrieve files over IPFS and save them locally.</description>
 </item>

Copy link
Contributor

@aschmahmann aschmahmann left a comment

Choose a reason for hiding this comment

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

I'm a little concerned that this diff is going to show up in every PR we make and cause noise. The lastBuildDate is always going to be updated and is something we'd just have to ignore, but maybe is livable. Is it a huge pain to ignore that one part of the diff?

If #330 continues to be a problem and we show a huge diff on every PR that's pretty sure to start bothering people. I can give it a try along with the go-ipfs v0.9.1 release though and see how that goes (hopefully building in docker resolves #330 and makes this not a problem).

Worst comes to worst we can make this action triggerable by command instead of happening on every PR.

scripts/diff.js Show resolved Hide resolved
@BigLep BigLep linked an issue Jul 20, 2021 that may be closed by this pull request
@guseggert
Copy link
Contributor Author

Is it a huge pain to ignore that one part of the diff?

No that's not hard, I'll add that.

The dates change on every build, even if no content changes, so to
reduce noise, we ignore those dates that are expected to change.

Also show the files that changed but for which we won't show diffs,
because the diffs are unreadable.
@github-actions
Copy link

This change produced no new differences in built artifacts.

@guseggert
Copy link
Contributor Author

You can see above that the dates are now skipped and it just leaves a "nothing changed" message.

Here's from a sample PR that shows what it now looks like when there are substantive changes:

Diff of Changes

The following files changed but will not show diffs, because the diffs would be unreadable:

  • site.css
  • site.js
diff -u --recursive --ignore-matching-lines '^\s*<pubDate>.*</pubDate>\s*' --ignore-matching-lines '^\s*<lastBuildDate>.*</lastBuildDate>\s*' a/404.html b/404.html
--- a/404.html 2021-07-21 16:16:07.022398201 +0000
+++ b/404.html 2021-07-21 16:16:07.158405984 +0000
@@ -1,2 +1,3 @@
 <h1>404</h1>
 <h3>Whoops. Looks like what you're looking for can't be found.</h3>
+this is some new text
diff -u --recursive --ignore-matching-lines '^\s*<pubDate>.*</pubDate>\s*' --ignore-matching-lines '^\s*<lastBuildDate>.*</lastBuildDate>\s*' a/index.html b/index.html
--- a/index.html 2021-07-21 16:16:07.314414911 +0000
+++ b/index.html 2021-07-21 16:16:07.426421320 +0000
@@ -30,6 +30,7 @@
 Available as RSS</a>
 </div>
 <h4 id="welcome-to-ipfs-distributions">Welcome to IPFS Distributions</h4>
+<p>This is some new text.</p>
 <p>This is the downloads website for all the official software distributions of the
 <a href="https://ipfs.io">IPFS Project</a>. You can find all the apps, binaries, and
 packages here. Every distribution has a section on this page with &hellip;</p>

scripts/diff.js Outdated
'-u',
'--recursive',
// these change on every build, so they just add noise, which is why we ignore them
'--ignore-matching-lines', '^\\s*<pubDate>.*</pubDate>\\s*',
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't need to ignore this one since any noise here is probably due to a bug (e.g. #330)

@github-actions
Copy link

Diff of Changes

The following files changed but will not show diffs, because the diffs would be unreadable:

  • site.css
  • site.js
diff -u --recursive --ignore-matching-lines '^\s*<lastBuildDate>.*</lastBuildDate>\s*' a/index.xml b/index.xml
--- a/index.xml 2021-07-21 17:30:43.482677296 +0000
+++ b/index.xml 2021-07-21 17:30:43.642679204 +0000
@@ -15,7 +15,7 @@
 <item>
 <title>fs-repo-migrations</title>
 <link>https://dist.ipfs.io/#fs-repo-migrations</link>
- <pubDate>Tue, 30 Mar 2021 22:00:00 GMT</pubDate>
+ <pubDate>Wed, 31 Mar 2021 00:00:00 GMT</pubDate>
 <description>fs-repo-migrations is a tool for migrating IPFS storage repositories to newer versions.</description>
 </item>
 
@@ -23,7 +23,7 @@
 <item>
 <title>go-ipfs</title>
 <link>https://dist.ipfs.io/#go-ipfs</link>
- <pubDate>Tue, 22 Jun 2021 22:00:00 GMT</pubDate>
+ <pubDate>Wed, 23 Jun 2021 00:00:00 GMT</pubDate>
 <description><p>go-ipfs is the main implementation of IPFS. It includes:</p>
 <ul>
 <li>an IPFS core implementation</li>
@@ -39,7 +39,7 @@
 <item>
 <title>gx</title>
 <link>https://dist.ipfs.io/#gx</link>
- <pubDate>Wed, 06 Mar 2019 23:00:00 GMT</pubDate>
+ <pubDate>Thu, 07 Mar 2019 00:00:00 GMT</pubDate>
 <description>gx is a package management tool built on ipfs</description>
 </item>
 
@@ -47,7 +47,7 @@
 <item>
 <title>gx-go</title>
 <link>https://dist.ipfs.io/#gx-go</link>
- <pubDate>Sun, 07 Oct 2018 22:00:00 GMT</pubDate>
+ <pubDate>Mon, 08 Oct 2018 00:00:00 GMT</pubDate>
 <description>gx-go is a gx helper tool for golang</description>
 </item>
 
@@ -55,7 +55,7 @@
 <item>
 <title>ipfs-cluster-ctl</title>
 <link>https://dist.ipfs.io/#ipfs-cluster</link>
- <pubDate>Wed, 07 Jul 2021 22:00:00 GMT</pubDate>
+ <pubDate>Thu, 08 Jul 2021 00:00:00 GMT</pubDate>
 <description>IPFS Cluster allows to allocate, replicate and track Pins across a cluster of IPFS daemons. ipfs-cluster-ctl is the command-line interface to manage a Cluster peer (run by ipfs-cluster-service).</description>
 </item>
 
@@ -63,7 +63,7 @@
 <item>
 <title>ipfs-cluster-follow</title>
 <link>https://dist.ipfs.io/#ipfs-cluster</link>
- <pubDate>Wed, 07 Jul 2021 22:00:00 GMT</pubDate>
+ <pubDate>Thu, 08 Jul 2021 00:00:00 GMT</pubDate>
 <description>ipfs-cluster-follow runs an IPFS Cluster follower peer, allowing users to easily join and be part of collaborative clusters.</description>
 </item>
 
@@ -71,7 +71,7 @@
 <item>
 <title>ipfs-cluster-service</title>
 <link>https://dist.ipfs.io/#ipfs-cluster</link>
- <pubDate>Wed, 07 Jul 2021 22:00:00 GMT</pubDate>
+ <pubDate>Thu, 08 Jul 2021 00:00:00 GMT</pubDate>
 <description>IPFS Cluster allows to allocate, replicate and track Pins across a cluster of IPFS daemons. ipfs-cluster-service runs a full Cluster peer.</description>
 </item>
 
@@ -79,7 +79,7 @@
 <item>
 <title>ipfs-ds-convert</title>
 <link>https://dist.ipfs.io/#ipfs-ds-convert</link>
- <pubDate>Wed, 17 Feb 2021 23:00:00 GMT</pubDate>
+ <pubDate>Thu, 18 Feb 2021 00:00:00 GMT</pubDate>
 <description>ipfs-ds-convert is a tool for migrating data between various IPFS datastore configurations</description>
 </item>
 
@@ -87,7 +87,7 @@
 <item>
 <title>ipfs-pack</title>
 <link>https://dist.ipfs.io/#ipfs-pack</link>
- <pubDate>Sun, 10 Dec 2017 23:00:00 GMT</pubDate>
+ <pubDate>Mon, 11 Dec 2017 00:00:00 GMT</pubDate>
 <description>A filesystem packing tool</description>
 </item>
 
@@ -95,7 +95,7 @@
 <item>
 <title>ipfs-see-all</title>
 <link>https://dist.ipfs.io/#ipfs-see-all</link>
- <pubDate>Mon, 10 Oct 2016 22:00:00 GMT</pubDate>
+ <pubDate>Tue, 11 Oct 2016 00:00:00 GMT</pubDate>
 <description>A diagnostics and recovery tool for ipfs repos.</description>
 </item>
 
@@ -103,7 +103,7 @@
 <item>
 <title>ipfs-update</title>
 <link>https://dist.ipfs.io/#ipfs-update</link>
- <pubDate>Tue, 30 Mar 2021 22:00:00 GMT</pubDate>
+ <pubDate>Wed, 31 Mar 2021 00:00:00 GMT</pubDate>
 <description>ipfs-update is a CLI tool to help update and install IPFS easily.</description>
 </item>
 
@@ -111,7 +111,7 @@
 <item>
 <title>ipget</title>
 <link>https://dist.ipfs.io/#ipget</link>
- <pubDate>Tue, 20 Apr 2021 22:00:00 GMT</pubDate>
+ <pubDate>Wed, 21 Apr 2021 00:00:00 GMT</pubDate>
 <description>wget for IPFS: retrieve files over IPFS and save them locally.</description>
 </item>
 

I had assumed that 'ipfs object diff' only returned files but it can
return directories too, in which case 'ipfs cat' fails, so this
switches (back) to 'ipfs get' to recursively fetch the contents of the
changed directories.

Also throw an exception when the 'diff' command exits with
code=2 (indicates some unexpected error).
Because of snap confinement, we can't use 'ipfs get -o' and then
access those files outside of the snap.
@github-actions
Copy link

This change produced no new differences in built artifacts.

@thattommyhall
Copy link
Member

lastBuildDate thing is potentially better fixed by #370

@BigLep BigLep requested a review from lidel July 27, 2021 15:49
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Thanks, this is very useful PR in the context of us moving to releases driven by PRs in this repo 👍

We can't merge this yet, as other things need to land first:

I will be doing this dance next week, and can take care of refactoring (I already have all the context to do it without much pain).
Sidenote: we switched from npm install to npm ci in #367, but not sure if it is enough to stabilize css and js minification

scripts/diff.js Outdated Show resolved Hide resolved
scripts/diff.js Outdated Show resolved Hide resolved
guseggert and others added 2 commits July 30, 2021 18:30
Co-authored-by: Marcin Rataj <lidel@lidel.org>
Co-authored-by: Marcin Rataj <lidel@lidel.org>
@github-actions
Copy link

Diff of Changes

diff --new-file -u --recursive a/index.xml b/index.xml
--- a/index.xml 2021-07-30 18:32:18.162506976 +0000
+++ b/index.xml 2021-07-30 18:32:18.110505993 +0000
@@ -6,7 +6,7 @@
 <description>Recent releases on IPFS Distributions</description>
 <generator>Hugo -- gohugo.io</generator>
 <language>en-us</language>
- <lastBuildDate>Wed, 21 Jul 2021 17:22:21 +0000</lastBuildDate>
+ <lastBuildDate>Fri, 30 Jul 2021 18:32:17 +0000</lastBuildDate>
 
 <atom:link href="https://dist.ipfs.io/index.xml" rel="self" type="application/rss&#43;xml" />
 

@github-actions
Copy link

github-actions bot commented Aug 4, 2021

Diff of Changes

diff --new-file -u --recursive a/index.xml b/index.xml
--- a/index.xml 2021-08-04 03:07:21.155328209 +0000
+++ b/index.xml 2021-08-04 03:07:21.095327549 +0000
@@ -6,7 +6,7 @@
 <description>Recent releases on IPFS Distributions</description>
 <generator>Hugo -- gohugo.io</generator>
 <language>en-us</language>
- <lastBuildDate>Wed, 21 Jul 2021 17:22:21 +0000</lastBuildDate>
+ <lastBuildDate>Wed, 21 Jul 2021 00:00:00 GMT</lastBuildDate>
 
 <atom:link href="https://dist.ipfs.io/index.xml" rel="self" type="application/rss&#43;xml" />
 

@github-actions
Copy link

github-actions bot commented Aug 4, 2021

Diff of Changes

Old: /ipns/dist.ipfs.io at /ipfs/QmX2KTMXiUb6tHTMS1JynhyQaL9qoj6uf1SLzHPhzGNnDv
New: /ipfs/QmUjMfTG5DUv3t4PJi73rovMzhqJaRSiGfQwTRtJiAXtx6

diff --new-file -u --recursive a/index.xml b/index.xml
--- a/index.xml 2021-08-04 12:21:11.273936144 +0000
+++ b/index.xml 2021-08-04 12:21:11.229935484 +0000
@@ -6,7 +6,7 @@
 <description>Recent releases on IPFS Distributions</description>
 <generator>Hugo -- gohugo.io</generator>
 <language>en-us</language>
- <lastBuildDate>Wed, 21 Jul 2021 17:22:21 +0000</lastBuildDate>
+ <lastBuildDate>Wed, 21 Jul 2021 00:00:00 GMT</lastBuildDate>
 
 <atom:link href="https://dist.ipfs.io/index.xml" rel="self" type="application/rss&#43;xml" />
 

@github-actions
Copy link

github-actions bot commented Aug 4, 2021

Diff of Changes

Old: /ipns/dist.ipfs.io at /ipfs/QmX2KTMXiUb6tHTMS1JynhyQaL9qoj6uf1SLzHPhzGNnDv
New: /ipfs/QmUjMfTG5DUv3t4PJi73rovMzhqJaRSiGfQwTRtJiAXtx6

diff --new-file -u --recursive a/index.xml b/index.xml
--- a/index.xml 2021-08-04 12:46:20.521518489 +0000
+++ b/index.xml 2021-08-04 12:46:20.457518095 +0000
@@ -6,7 +6,7 @@
 <description>Recent releases on IPFS Distributions</description>
 <generator>Hugo -- gohugo.io</generator>
 <language>en-us</language>
- <lastBuildDate>Wed, 21 Jul 2021 17:22:21 +0000</lastBuildDate>
+ <lastBuildDate>Wed, 21 Jul 2021 00:00:00 GMT</lastBuildDate>
 
 <atom:link href="https://dist.ipfs.io/index.xml" rel="self" type="application/rss&#43;xml" />
 

@ipfs ipfs deleted a comment from github-actions bot Aug 4, 2021
@ipfs ipfs deleted a comment from github-actions bot Aug 4, 2021
this removes a third-party action, bit safer
@github-actions
Copy link

github-actions bot commented Aug 4, 2021

Diff of Changes

Old: /ipns/dist.ipfs.io at /ipfs/QmX2KTMXiUb6tHTMS1JynhyQaL9qoj6uf1SLzHPhzGNnDv
New: /ipfs/QmUjMfTG5DUv3t4PJi73rovMzhqJaRSiGfQwTRtJiAXtx6

diff --new-file -u --recursive a/index.xml b/index.xml
--- a/index.xml 2021-08-04 14:05:23.426986259 +0000
+++ b/index.xml 2021-08-04 14:05:23.346983743 +0000
@@ -6,7 +6,7 @@
 <description>Recent releases on IPFS Distributions</description>
 <generator>Hugo -- gohugo.io</generator>
 <language>en-us</language>
- <lastBuildDate>Wed, 21 Jul 2021 17:22:21 +0000</lastBuildDate>
+ <lastBuildDate>Wed, 21 Jul 2021 00:00:00 GMT</lastBuildDate>
 
 <atom:link href="https://dist.ipfs.io/index.xml" rel="self" type="application/rss&#43;xml" />
 

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Thanks!

I've rebased this on top of signed builds, merging.

@lidel lidel merged commit 02d5c56 into master Aug 4, 2021
@lidel lidel deleted the feat/deep-diff branch August 4, 2021 14:09
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.

Produce easier to digest diff of updates
4 participants