rubyatscale/pack_stats

Proposed minor change to example in README file

Closed this issue ยท 4 comments

https://github.com/rubyatscale/modularization_statistics/blob/e99fc4a500a6c4972636b41d54f84ed4b4e56089/README.md#tracking-privacy-and-dependency-violations-reliably

Currently the "Tracking Privacy and Dependency Violations Reliably" example in the readme does the following:

  • Uses ParsePackwerk.all to build an array old_packages
  • Loops through old_packages and builds a new ParsePackwerk::Package with enforce_dependencies and enforce_privacy set true. Writes that package in yml.
  • Runs packwerk update-deprecations
  • Loops through old_packages and builds a new ParsePackwerk::Package with the original settings, and writes that package in yml again

That all makes sense to me. We change enforce_dependencies and enforce_privacy to true, then run packwerk update-depredations, and then revert the change.

I've a specific question about the last fragment, though:

    # Now we reset it back so that the protection values are the same as the native packwerk configuration
    old_packages.each do |package|
      new_package = ParsePackwerk::Package.new(
        dependencies: package.dependencies,
        enforce_dependencies: package.enforce_dependencies,
        enforce_privacy: package.enforce_privacy,
        metadata: package.metadata,
        name: package.name
      )
      ParsePackwerk.write_package_yml!(new_package)
    end

Is there a reason we've not done this instead? In the example code ParsePackwerk::Package.new is just building a brand-new ParsePackwerk::Package object with the same parameters as it originally had, right?

old_packages.each do |old_package|
  ParsePackwerk.write_package_yml!(old_package)
end

I think you're right @oskarpearson -- we can simply do as you suggested and rewrite the old packages, as they are immutable and we do not need to redundantly reinitialize each one. Thanks for catching this! Want to send a PR to update it?

Will do once #18 is merged since otherwise it's going to create PR conflicts

Thanks for suggesting this change @oskarpearson ๐Ÿ™Œ !

Thanks for the fix