From bb841a38b3ba22fb6a5c3c81f35ef29debaae0cd Mon Sep 17 00:00:00 2001 From: Dave Corson-Knowles Date: Wed, 4 Feb 2026 02:43:49 -0800 Subject: [PATCH 1/3] Enable order preservation --- lib/parse_packwerk.rb | 33 ++++++- lib/parse_packwerk/package.rb | 7 +- spec/parse_packwerk_spec.rb | 161 ++++++++++++++++++++++++++++++++++ 3 files changed, 198 insertions(+), 3 deletions(-) diff --git a/lib/parse_packwerk.rb b/lib/parse_packwerk.rb index 39e9246..bca4329 100644 --- a/lib/parse_packwerk.rb +++ b/lib/parse_packwerk.rb @@ -31,6 +31,19 @@ def initialize(packwerk_file_name) extend T::Sig + # Configuration option to preserve the original key order when writing package.yml files. + # When true, keys will be written in their original order from the file rather than + # being sorted according to key_sort_order. This reduces diff churn when tools modify packages. + # Defaults to false for backwards compatibility. + @preserve_key_order = T.let(false, T::Boolean) + + class << self + extend T::Sig + + sig { returns(T::Boolean) } + attr_accessor :preserve_key_order + end + sig do returns(T::Array[Package]) end @@ -90,8 +103,7 @@ def self.write_package_yml!(package) merged_config.merge!('metadata' => package.metadata) end - sorted_keys = key_sort_order - merged_config = merged_config.to_a.sort_by { |key, _value| T.unsafe(sorted_keys).index(key) || 1000 }.to_h + merged_config = sort_keys(merged_config, package.original_key_order) raw_yaml = YAML.dump(merged_config) stylized_yaml = raw_yaml.gsub("---\n", '') @@ -99,6 +111,23 @@ def self.write_package_yml!(package) end end + sig { params(config: T::Hash[T.untyped, T.untyped], original_key_order: T::Array[String]).returns(T::Hash[T.untyped, T.untyped]) } + def self.sort_keys(config, original_key_order) + if preserve_key_order && original_key_order.any? + # Preserve original key order: existing keys stay in their original position, + # new keys are appended in the default sort order + existing_keys = original_key_order & config.keys + new_keys = config.keys - original_key_order + sorted_new_keys = new_keys.sort_by { |key| key_sort_order.index(key) || 1000 } + + ordered_keys = existing_keys + sorted_new_keys + ordered_keys.each_with_object({}) { |key, hash| hash[key] = config[key] if config.key?(key) } + else + # Default behavior: sort by canonical key order + config.to_a.sort_by { |key, _value| T.unsafe(key_sort_order).index(key) || 1000 }.to_h + end + end + sig { returns(T::Array[String]) } def self.key_sort_order %w[ diff --git a/lib/parse_packwerk/package.rb b/lib/parse_packwerk/package.rb index fa2ad35..c40b1c0 100644 --- a/lib/parse_packwerk/package.rb +++ b/lib/parse_packwerk/package.rb @@ -13,11 +13,15 @@ class Package < T::Struct const :dependencies, T::Array[String] const :config, T::Hash[T.untyped, T.untyped] const :violations, T::Array[Violation] + # Stores the original key order from the YAML file for preserving order on write + const :original_key_order, T::Array[String], default: [] sig { params(pathname: Pathname).returns(Package) } def self.from(pathname) package_loaded_yml = YAML.load_file(pathname) || {} package_name = pathname.dirname.cleanpath.to_s + # Capture the original key order from the YAML file + original_keys = package_loaded_yml.is_a?(Hash) ? package_loaded_yml.keys : [] new( name: package_name, @@ -28,7 +32,8 @@ def self.from(pathname) metadata: package_loaded_yml[METADATA] || {}, dependencies: package_loaded_yml[DEPENDENCIES] || [], config: package_loaded_yml, - violations: PackageTodo.from(PackageTodo.yml(directory(package_name))).violations + violations: PackageTodo.from(PackageTodo.yml(directory(package_name))).violations, + original_key_order: original_keys ) end diff --git a/spec/parse_packwerk_spec.rb b/spec/parse_packwerk_spec.rb index 7ffd115..30b2fe2 100644 --- a/spec/parse_packwerk_spec.rb +++ b/spec/parse_packwerk_spec.rb @@ -1201,5 +1201,166 @@ def pack_as_hash(package) expect(pack_as_hash(all_packages.first)).to eq pack_as_hash(package) end end + + describe 'preserve_key_order option' do + after do + ParsePackwerk.preserve_key_order = false + end + + context 'when preserve_key_order is false (default)' do + before do + ParsePackwerk.preserve_key_order = false + write_file(package_yml, <<~CONTENTS) + enforce_privacy: true + enforce_layers: true + layer: admin + enforce_dependencies: true + dependencies: + - packs/foo + CONTENTS + end + + it 'reorders keys according to key_sort_order' do + package = ParsePackwerk::Package.from(package_yml) + ParsePackwerk.write_package_yml!(package) + + expect(package_yml.read).to eq <<~PACKAGEYML + enforce_dependencies: true + enforce_privacy: true + enforce_layers: true + layer: admin + dependencies: + - packs/foo + PACKAGEYML + end + end + + context 'when preserve_key_order is true' do + before do + ParsePackwerk.preserve_key_order = true + write_file(package_yml, <<~CONTENTS) + enforce_privacy: true + enforce_layers: true + layer: admin + enforce_dependencies: true + dependencies: + - packs/foo + CONTENTS + end + + it 'preserves the original key order from the file' do + package = ParsePackwerk::Package.from(package_yml) + ParsePackwerk.write_package_yml!(package) + + expect(package_yml.read).to eq <<~PACKAGEYML + enforce_privacy: true + enforce_layers: true + layer: admin + enforce_dependencies: true + dependencies: + - packs/foo + PACKAGEYML + end + + it 'appends new keys in canonical order at the end' do + package = ParsePackwerk::Package.from(package_yml) + # Add metadata which wasn't in the original file + new_package = package.with(metadata: { 'owner' => 'Team A' }) + ParsePackwerk.write_package_yml!(new_package) + + expect(package_yml.read).to eq <<~PACKAGEYML + enforce_privacy: true + enforce_layers: true + layer: admin + enforce_dependencies: true + dependencies: + - packs/foo + metadata: + owner: Team A + PACKAGEYML + end + + it 'handles removed keys gracefully' do + package = ParsePackwerk::Package.from(package_yml) + # Remove dependencies + new_package = package.with(dependencies: []) + ParsePackwerk.write_package_yml!(new_package) + + expect(package_yml.read).to eq <<~PACKAGEYML + enforce_privacy: true + enforce_layers: true + layer: admin + enforce_dependencies: true + PACKAGEYML + end + end + + context 'when preserve_key_order is true but package has no original_key_order' do + before do + ParsePackwerk.preserve_key_order = true + end + + it 'falls back to canonical key order for new packages' do + package = build_pack(dependencies: ['packs/foo']) + ParsePackwerk.write_package_yml!(package) + + expect(package_yml.read).to eq <<~PACKAGEYML + enforce_dependencies: true + enforce_privacy: true + enforce_layers: true + dependencies: + - packs/foo + PACKAGEYML + end + end + + context 'when package is modified after reading' do + before do + ParsePackwerk.preserve_key_order = true + write_file(package_yml, <<~CONTENTS) + enforce_privacy: true + enforce_layers: true + layer: admin + enforce_dependencies: true + dependencies: + - packs/foo + CONTENTS + end + + it 'preserves key order even when dependencies change' do + package = ParsePackwerk::Package.from(package_yml) + # Change dependencies value but keep the key + new_package = package.with(dependencies: ['packs/bar', 'packs/baz']) + ParsePackwerk.write_package_yml!(new_package) + + expect(package_yml.read).to eq <<~PACKAGEYML + enforce_privacy: true + enforce_layers: true + layer: admin + enforce_dependencies: true + dependencies: + - packs/bar + - packs/baz + PACKAGEYML + end + end + + context 'preserves original key order when reading and writing package' do + before do + ParsePackwerk.preserve_key_order = true + end + + it 'stores original_key_order when reading package' do + write_file(package_yml, <<~CONTENTS) + enforce_privacy: true + layer: admin + enforce_dependencies: true + CONTENTS + + package = ParsePackwerk::Package.from(package_yml) + expect(package.original_key_order).to eq ['enforce_privacy', 'layer', 'enforce_dependencies'] + end + end + end end end From 58fd02ee7cfea00883201fa3de7d85fe81b0c5cc Mon Sep 17 00:00:00 2001 From: Dave Corson-Knowles Date: Wed, 4 Feb 2026 11:05:30 -0800 Subject: [PATCH 2/3] Update parse_packwerk_spec.rb --- spec/parse_packwerk_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/parse_packwerk_spec.rb b/spec/parse_packwerk_spec.rb index 30b2fe2..2280874 100644 --- a/spec/parse_packwerk_spec.rb +++ b/spec/parse_packwerk_spec.rb @@ -1358,7 +1358,7 @@ def pack_as_hash(package) CONTENTS package = ParsePackwerk::Package.from(package_yml) - expect(package.original_key_order).to eq ['enforce_privacy', 'layer', 'enforce_dependencies'] + expect(package.original_key_order).to eq %w[enforce_privacy layer enforce_dependencies] end end end From f0d9bd8b32a5e6598cdf75c1d3e003a5c6226ab7 Mon Sep 17 00:00:00 2001 From: Dave Corson-Knowles Date: Wed, 4 Feb 2026 11:07:04 -0800 Subject: [PATCH 3/3] Bump minor version --- Gemfile.lock | 2 +- parse_packwerk.gemspec | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 1b3a486..2263d0b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - parse_packwerk (0.26.1) + parse_packwerk (0.27.0) bigdecimal sorbet-runtime diff --git a/parse_packwerk.gemspec b/parse_packwerk.gemspec index 491b4a6..913a052 100644 --- a/parse_packwerk.gemspec +++ b/parse_packwerk.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |spec| spec.name = 'parse_packwerk' - spec.version = '0.26.1' + spec.version = '0.27.0' spec.authors = ['Gusto Engineers'] spec.email = ['dev@gusto.com'] spec.summary = 'A low-dependency gem for parsing and writing packwerk YML files'