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

piezo 1.9.1 #169019

Merged
merged 3 commits into from Apr 18, 2024
Merged

piezo 1.9.1 #169019

merged 3 commits into from Apr 18, 2024

Conversation

pthariensflame
Copy link
Contributor

Important: Do not tick a checkbox if you haven’t performed its action. Honesty is indispensable for a smooth review process.

In the following questions <cask> is the token of the cask you're submitting.

After making any changes to a cask, existing or new, verify:


Only on 14+ (should be only on 14.4+ but we don't have anything finer-grained AFAIK).

Casks/p/piezo.rb Outdated
Comment on lines 4 to 18
on_ventura :or_older do
version "1.8.2"
end
on_sonoma :or_newer do
version "1.9.0"
end

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that the system version in the livecheck url indicates the mac os version in numbers. Most recent sonoma version is 14.4.0 and so it should be separated. The version parameter do nothing I guess. The later livecheck should be deleted if you apply this change.

Suggested change
on_ventura :or_older do
version "1.8.2"
end
on_sonoma :or_newer do
version "1.9.0"
end
on_ventura :or_older do
version "1.8.2"
livecheck do
url "https://rogueamoeba.net/ping/versionCheck.cgi?format=sparkle&system=1231&bundleid=com.rogueamoeba.Piezo&platform=osx"
strategy :sparkle
end
end
on_sonoma :or_newer do
version "1.9.0"
livecheck do
url "https://rogueamoeba.net/ping/versionCheck.cgi?format=sparkle&system=1440&bundleid=com.rogueamoeba.Piezo&platform=osx"
strategy :sparkle
end
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Btw, sonoma with 14.3.1 will get the wrong version, so maybe it looks better to remain it until next major version update?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I just wasn't sure if someone would come here and know an incantation to actually pick out 14.4+ within a cask.

Copy link
Member

Choose a reason for hiding this comment

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

We don't support scoping to minor/patch versions of macOS. It's presumed that users are on the latest minor/patch version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@p-linnane Is there a way to obtain that number, so it can be slotted into the livecheck URL?

Copy link
Member

Choose a reason for hiding this comment

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

Not reliably to my knowledge. @Bo98 any ideas if there's a clever way to handle this?

Copy link
Member

@Bo98 Bo98 Mar 19, 2024

Choose a reason for hiding this comment

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

MacOS.full_version exists but it is maybe not what you want. We don't store information on the latest patch release per major OS.

Copy link
Member

Choose a reason for hiding this comment

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

The version that is in the livecheck won't affect what version is installed anyway. The new version will be installed on any machine running Sonoma.

@daeho-ro
Copy link
Contributor

What about in this way with the custom module?

module Utils
  def self.reduced_version
    MacOS.full_version.to_s.delete(".")[0, 3]
  end

  def self.piezo_version
    if reduced_version < "144"
      "1.8.2"
    else
      "1.9.0"
    end
  end
end

cask "piezo" do
  version Utils.piezo_version.to_s
  sha256 :no_check

  url "https://rogueamoeba.com/piezo/download/Piezo.zip"
  name "Piezo"
  desc "Audio recording application"
  homepage "https://rogueamoeba.com/piezo/"

  livecheck do
    url "https://rogueamoeba.net/ping/versionCheck.cgi?format=sparkle&system=#{Utils.reduced_version}&bundleid=com.rogueamoeba.Piezo&platform=osx"
    strategy :sparkle
  end

 ...

Copy link

github-actions bot commented Apr 9, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale Issue which has not received any feedback for some time. label Apr 9, 2024
@pthariensflame
Copy link
Contributor Author

I'll come back to this; there are now more apps from Rogue Amoeba that have this exact same issue.

@krehel krehel mentioned this pull request Apr 13, 2024
9 tasks
@samford samford force-pushed the piezo-1.9 branch 2 times, most recently from 353e46e to 85f5513 Compare April 14, 2024 12:18
Copy link
Member

@samford samford left a comment

Choose a reason for hiding this comment

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

I've updated this to use the setup that I recently used in airfoil and audio-hijack, while also updating the cask for 1.9.1 (and rebasing the branch). This should bring us closer to where we want to be but there may still be something to be said for discussing this approach (in case there's still room for improvement).

I believe the open questions are:

  • Should we use MacOS.full_version.to_s.delete(".") instead of hardcoding the system value (in the livecheck block url)? This will vary based on the execution environment, so there's a trade-off between maintainability (having to periodically update the system value) and predictability. I'm not sure that anyone will remember to update the system value over time (which will quietly cause the check to stop returning new versions), so a dynamic approach may make more sense.
  • Is there any benefit to having the version query string parameter in the livecheck block url? The value we're using doesn't seem to correspond with the value the application uses when fetching the Sparkle feed. We can experiment with omitting the version value and if that continues to give us the newest appropriate version over time, then that may be fine.

For what it's worth, I've opted to use 1366 as the system value for the on_ventura :or_older block, as using the highest macOS version in that range will hopefully keep that check working for longer. If we use the lowest version in the range, then the check may quietly stop returning newer versions at an earlier point if/when upstream increases the macOS requirement for the older variant in the future. For example, if we used 11 and they only dropped Big Sur support, then the check would always return an older version even if newer versions appear.

@samford samford changed the title piezo 1.9.0 piezo 1.9.1 Apr 14, 2024
@samford samford removed the stale Issue which has not received any feedback for some time. label Apr 14, 2024
Casks/p/piezo.rb Outdated Show resolved Hide resolved
Casks/p/piezo.rb Outdated Show resolved Hide resolved
pthariensflame and others added 3 commits April 16, 2024 22:08
Only on 14+ (should be only on 14.4+ but we don't have anything finer-grained AFAIK).
The newer version of Piezo that require macOS 14.4 is
`Piezo-ARK.zip` instead of `Piezo.zip` (the older variant).

Co-authored-by: Daeho Ro <[email protected]>
@razvanazamfirei razvanazamfirei merged commit 685aaab into Homebrew:master Apr 18, 2024
15 checks passed
@pthariensflame pthariensflame deleted the piezo-1.9 branch April 18, 2024 23:54
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

8 participants