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

Refactor *VersionHelpers to use an object #203

Open
AliSoftware opened this issue Feb 2, 2021 · 2 comments
Open

Refactor *VersionHelpers to use an object #203

AliSoftware opened this issue Feb 2, 2021 · 2 comments

Comments

@AliSoftware
Copy link
Contributor

Today ios_version_helper and android_version_helper consist of freeform, module-static methods which take various and heterogeneous types of parameters depending on the method (sometimes a String for versionName, sometimes a Hash, etc)

It would make way more sense, and make the code way easier to understand, if instead of freeform methods, we created a class Version which would expose the appropriate instance properties (e.g. for Android, name and code) and instance methods to manipulate those versions, inspired by Gem::Version and its methods.

This would make the implementation way easier to understand and document (not having to repeat the parameter type documentation methods taking similar inputs), but also clearer at call-site and more idiomatic.


Some ideas / inspiration for possible API, e.g. for the Android one in the Fastlane::Helper::Android module:

class Version
  # [String] Doc here
  attr_reader :name
  # [Int] Doc here
  attr_reader :code

  # @return [Version] Next release version
  def next_release
    
  end
  
end

Or, alternatively, something like:

class Version
  attr_reader :major, :minor, :hotfix
  attr_reader :is_alpha?
  attr_reader :suffix
  attr_reader :code

  # @return [String] The versionName. `"alpha-major.minor.patch-suffix"` if `is_alpha?`, `"major.minor.patch-rc-suffix"` otherwise.
  def name
    (sprefix, *ssuffixes) = is_alpha? ? ["alpha", suffix] : [nil, "rc", suffix]
    [sprefix, "#{major}.#{minor}.#{patch}", *ssuffixes].compact.join('-')
  end
  
  # And a lot of instance methods to bump versions up and down, etc
end
@AliSoftware
Copy link
Contributor Author

Another benefit of refactoring this to be objects, and to have instance methods on them, is that it would be easier to refactor that to not rely on the various env vars like ENV['XX_CONFIG_FILE'], and to instead take the paths for those config files as parameter to the constructor of those Version objects. Which would then allow us to easily call both getters and setters (get current version, set new version for upcoming beta or release, etc) on the instance without relying to any global state

@AliSoftware
Copy link
Contributor Author

Some work have already been started in the 203-version branch here to work on this refactoring.

Currently, only the model for Android versioning have been started to be worked on (and haven't been fully tested), so there's still work to do in order to apply the same ideas for iOS version objects.

This migration/refactoring would also make it easier to support multiple apps, in particular easily support both WordPress and Jetpack in the same repository, by passing the right parameters (Jetpack or WP config file paths) when constructing the VersionSet instance, at which point we could treat the VersionSet the same both for Jetpack and WordPress (calls to get and set version, etc) after the init point

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

No branches or pull requests

1 participant