From c212f60715afbbf4f6d1ca16b7453b1e5913c230 Mon Sep 17 00:00:00 2001 From: Ashley Willard Date: Tue, 17 Dec 2024 13:18:07 -0800 Subject: [PATCH 1/5] change add readme todo to add readme --- lib/packs/private.rb | 14 ++++++---- lib/packs/user_event_logger.rb | 2 +- spec/packs_spec.rb | 49 +++++++++++++++++++--------------- 3 files changed, 37 insertions(+), 28 deletions(-) diff --git a/lib/packs/private.rb b/lib/packs/private.rb index 20883e0..7117c37 100644 --- a/lib/packs/private.rb +++ b/lib/packs/private.rb @@ -69,7 +69,7 @@ def self.create_pack!(pack_name:, enforce_dependencies:, enforce_privacy:, enfor team: team ) add_public_directory(package) if package.enforce_privacy - add_readme_todo(package) + add_readme(package) Logging.section('Next steps') do next_steps = Packs.config.user_event_logger.after_create_pack(pack_name) @@ -140,7 +140,7 @@ def self.move_to_pack!(pack_name:, paths_relative_to_root:, per_file_processors: end end - add_readme_todo(package) + add_readme(package) per_file_processors.each do |processor| processor.after_move_files!(file_move_operations) @@ -452,12 +452,16 @@ def self.add_public_directory(package) end sig { params(package: ParsePackwerk::Package).void } - def self.add_readme_todo(package) + def self.add_readme(package) pack_directory = package.directory if !pack_directory.join('README.md').exist? - readme_todo_md = Packs.config.user_event_logger.on_create_readme_todo(package.name) - pack_directory.join('README_TODO.md').write(readme_todo_md) + readme_md = Packs.config.user_event_logger.on_create_readme(package.name) + pack_directory.join('README.md').write(readme_md) + + if pack_directory.join('README_TODO.md').exist? + pack_directory.join('README_TODO.md').delete + end end end diff --git a/lib/packs/user_event_logger.rb b/lib/packs/user_event_logger.rb index 9950e14..802c7ee 100644 --- a/lib/packs/user_event_logger.rb +++ b/lib/packs/user_event_logger.rb @@ -147,7 +147,7 @@ def on_create_public_directory_todo(pack_name) end sig { params(pack_name: String).returns(String) } - def on_create_readme_todo(pack_name) + def on_create_readme(pack_name) readme_template_pathname = Packs.config.readme_template_pathname readme_template = readme_template_pathname.read if readme_template_pathname.exist? diff --git a/spec/packs_spec.rb b/spec/packs_spec.rb index 91424b0..adc5cab 100644 --- a/spec/packs_spec.rb +++ b/spec/packs_spec.rb @@ -286,7 +286,7 @@ def write_codeownership_config end describe 'setting the README' do - let(:expected_readme_todo) do + let(:expected_readme) do <<~EXPECTED Welcome to `packs/organisms`! @@ -306,31 +306,34 @@ def write_codeownership_config EXPECTED end - it 'adds a README_TODO.md file as a placeholder' do + it 'adds a README.md file as a placeholder' do Packs.create_pack!(pack_name: 'packs/organisms') ParsePackwerk.bust_cache! - actual_readme_todo = ParsePackwerk.find('packs/organisms').directory.join('README_TODO.md') - expect(actual_readme_todo.read).to eq expected_readme_todo + actual_readme = ParsePackwerk.find('packs/organisms').directory.join('README.md') + expect(actual_readme.read).to eq expected_readme end context 'app has one pack with an outdated README_TODO.md' do it 'overwrites the README_TODO.md' do write_file('packs/organisms/README_TODO.md', 'This is outdated') write_package_yml('packs/organisms') - actual_readme_todo = ParsePackwerk.find('packs/organisms').directory.join('README_TODO.md') - expect(actual_readme_todo.read).to eq 'This is outdated' + readme_todo = ParsePackwerk.find('packs/organisms').directory.join('README_TODO.md') + expect(readme_todo.read).to eq 'This is outdated' + Packs.create_pack!(pack_name: 'packs/organisms') - expect(actual_readme_todo.read).to eq expected_readme_todo + actual_readme = ParsePackwerk.find('packs/organisms').directory.join('README.md') + expect(actual_readme.read).to eq expected_readme + expect(readme_todo.exist?).to eq false end end context 'app has one pack with a README.md' do - it 'does not add a README_TODO.md file' do + it 'does not change the README.md file' do write_package_yml('packs/organisms') - write_file('packs/organisms/README.md') - actual_readme_todo = ParsePackwerk.find('packs/organisms').directory.join('README_TODO.md') + write_file('packs/organisms/README.md', 'This is the original README.md') + actual_readme = ParsePackwerk.find('packs/organisms').directory.join('README.md') Packs.create_pack!(pack_name: 'packs/organisms') - expect(actual_readme_todo.exist?).to eq false + expect(actual_readme.read).to eq 'This is the original README.md' end end @@ -856,7 +859,7 @@ def write_codeownership_config end describe 'setting the README' do - let(:expected_readme_todo) do + let(:expected_readme) do <<~EXPECTED Welcome to `packs/organisms`! @@ -876,7 +879,7 @@ def write_codeownership_config EXPECTED end - it 'adds a README_TODO.md file as a placeholder' do + it 'adds a README.md file as a placeholder' do write_file('app/services/foo.rb') write_package_yml('packs/organisms') Packs.move_to_pack!( @@ -884,8 +887,8 @@ def write_codeownership_config paths_relative_to_root: ['app/services/foo.rb'] ) - actual_readme_todo = ParsePackwerk.find('packs/organisms').directory.join('README_TODO.md') - expect(actual_readme_todo.read).to eq expected_readme_todo + actual_readme = ParsePackwerk.find('packs/organisms').directory.join('README.md') + expect(actual_readme.read).to eq expected_readme end context 'app has one pack with an outdated README_TODO.md' do @@ -893,27 +896,29 @@ def write_codeownership_config write_file('app/services/foo.rb') write_package_yml('packs/organisms') write_file('packs/organisms/README_TODO.md', 'This is outdated') - actual_readme_todo = ParsePackwerk.find('packs/organisms').directory.join('README_TODO.md') - expect(actual_readme_todo.read).to eq 'This is outdated' + readme_todo = ParsePackwerk.find('packs/organisms').directory.join('README_TODO.md') + expect(readme_todo.read).to eq 'This is outdated' Packs.move_to_pack!( pack_name: 'packs/organisms', paths_relative_to_root: ['app/services/foo.rb'] ) - expect(actual_readme_todo.read).to eq expected_readme_todo + actual_readme = ParsePackwerk.find('packs/organisms').directory.join('README.md') + expect(actual_readme.read).to eq expected_readme + expect(readme_todo.exist?).to eq false end end context 'app has one pack with a README.md' do - it 'does not add a README_TODO.md file' do + it 'does not change the README.md file' do write_file('app/services/foo.rb') write_package_yml('packs/organisms') - write_file('packs/organisms/README.md') - actual_readme_todo = ParsePackwerk.find('packs/organisms').directory.join('README_TODO.md') + write_file('packs/organisms/README.md', 'This is the original README.md') + actual_readme = ParsePackwerk.find('packs/organisms').directory.join('README.md') Packs.move_to_pack!( pack_name: 'packs/organisms', paths_relative_to_root: ['app/services/foo.rb'] ) - expect(actual_readme_todo.exist?).to eq false + expect(actual_readme.read).to eq 'This is the original README.md' end end end From 3afecf5e71c7d2c7511f754731c688bfab488aab Mon Sep 17 00:00:00 2001 From: Ashley Willard Date: Tue, 17 Dec 2024 13:51:06 -0800 Subject: [PATCH 2/5] create a directory rather than a public/TODO.md file --- lib/packs/private.rb | 4 ++-- lib/packs/user_event_logger.rb | 23 ----------------------- spec/packs_spec.rb | 18 +++++++++--------- 3 files changed, 11 insertions(+), 34 deletions(-) diff --git a/lib/packs/private.rb b/lib/packs/private.rb index 7117c37..0e63de6 100644 --- a/lib/packs/private.rb +++ b/lib/packs/private.rb @@ -446,8 +446,8 @@ def self.add_public_directory(package) if public_directory.glob('**/**.rb').none? FileUtils.mkdir_p(public_directory) - todo_md = Packs.config.user_event_logger.on_create_public_directory_todo(package.name) - public_directory.join('TODO.md').write(todo_md) + package_name = package.directory.basename.to_s + FileUtils.mkdir_p(public_directory.join(package_name)) end end diff --git a/lib/packs/user_event_logger.rb b/lib/packs/user_event_logger.rb index 802c7ee..262436a 100644 --- a/lib/packs/user_event_logger.rb +++ b/lib/packs/user_event_logger.rb @@ -123,29 +123,6 @@ def after_move_to_folder(pack_name) MSG end - sig { params(pack_name: String).returns(String) } - def on_create_public_directory_todo(pack_name) - <<~MSG - This directory holds your public API! - - Any classes, constants, or modules that you want other packs to use and you intend to support should go in here. - Anything that is considered private should go in other folders. - - If another pack uses classes, constants, or modules that are not in your public folder, it will be considered a "privacy violation" by packwerk. - You can prevent other packs from using private API by using packwerk. - - Want to find how your private API is being used today? - Try running: `bin/packs list_top_violations privacy #{pack_name}` - - Want to move something into this folder? - Try running: `bin/packs make_public #{pack_name}/path/to/file.rb` - - One more thing -- feel free to delete this file and replace it with a README.md describing your package in the main package directory. - - See #{documentation_link} for more info! - MSG - end - sig { params(pack_name: String).returns(String) } def on_create_readme(pack_name) readme_template_pathname = Packs.config.readme_template_pathname diff --git a/spec/packs_spec.rb b/spec/packs_spec.rb index adc5cab..a1401e3 100644 --- a/spec/packs_spec.rb +++ b/spec/packs_spec.rb @@ -256,21 +256,22 @@ def write_codeownership_config end context 'app has no public dir' do - it 'adds a TODO.md file letting someone know what to do with it' do + it 'adds a public directory' do Packs.create_pack!(pack_name: 'packs/organisms') - actual_todo = Pathname.new('packs/organisms/app/public/TODO.md').read - expect(actual_todo).to eq expected_todo + public_directory = Pathname.new('packs/organisms/app/public') + expect(public_directory.exist?).to eq true + expect(public_directory.join('organisms').exist?).to eq true end context 'pack not enforcing privacy' do - it 'does not add a TODO.md file' do + it 'does not add a public directory' do Packs.create_pack!(pack_name: 'packs/organisms', enforce_privacy: false) ParsePackwerk.bust_cache! package = ParsePackwerk.find('packs/organisms') expect(package.enforce_privacy).to eq(false) - todo_file = Pathname.new('packs/organisms/app/public/TODO.md') - expect(todo_file.exist?).to eq false + public_directory = Pathname.new('packs/organisms/app/public') + expect(public_directory.exist?).to eq false end end end @@ -815,7 +816,7 @@ def write_codeownership_config end context 'app has no public dir' do - it 'adds a TODO.md file letting someone know what to do with it' do + it 'adds a public directory' do write_file('app/services/foo.rb') write_package_yml('packs/organisms') Packs.move_to_pack!( @@ -823,8 +824,7 @@ def write_codeownership_config paths_relative_to_root: ['app/services/foo.rb'] ) - actual_todo = Pathname.new('packs/organisms/app/public/TODO.md').read - expect(actual_todo).to eq expected_todo + expect(Pathname.new('packs/organisms/app/public/organisms').exist?).to eq true end context 'pack not enforcing privacy' do From 39184fd219e9059159f880c393c2a44e55824786 Mon Sep 17 00:00:00 2001 From: Ashley Willard Date: Thu, 2 Jan 2025 14:22:23 -0800 Subject: [PATCH 3/5] bump version --- Gemfile.lock | 2 +- packs.gemspec | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index c2a9423..b2a586f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -17,7 +17,7 @@ GIT PATH remote: . specs: - packs (0.1.0) + packs (0.2.0) bigdecimal code_ownership (>= 1.33.0) packs-specification diff --git a/packs.gemspec b/packs.gemspec index 0e909b5..be2089e 100644 --- a/packs.gemspec +++ b/packs.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |spec| spec.name = 'packs' - spec.version = '0.1.0' + spec.version = '0.2.0' spec.authors = ['Gusto Engineers'] spec.email = ['dev@gusto.com'] From 4044b9150a02469cf679f71470e5e45067eb494a Mon Sep 17 00:00:00 2001 From: ashleywillard Date: Thu, 2 Jan 2025 15:36:43 -0800 Subject: [PATCH 4/5] Update spec/packs_spec.rb Co-authored-by: Teal Stannard Update spec/packs_spec.rb Co-authored-by: Teal Stannard --- spec/packs_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/packs_spec.rb b/spec/packs_spec.rb index a1401e3..f31468e 100644 --- a/spec/packs_spec.rb +++ b/spec/packs_spec.rb @@ -315,7 +315,7 @@ def write_codeownership_config end context 'app has one pack with an outdated README_TODO.md' do - it 'overwrites the README_TODO.md' do + it 'deletes the README_TODO.md and adds a README' do write_file('packs/organisms/README_TODO.md', 'This is outdated') write_package_yml('packs/organisms') readme_todo = ParsePackwerk.find('packs/organisms').directory.join('README_TODO.md') @@ -892,7 +892,7 @@ def write_codeownership_config end context 'app has one pack with an outdated README_TODO.md' do - it 'overwrites the README_TODO.md' do + it 'deletes the README_TODO.md and adds a README' do write_file('app/services/foo.rb') write_package_yml('packs/organisms') write_file('packs/organisms/README_TODO.md', 'This is outdated') From 57fdc0430ec42622c4d97e1cc27861560a6a58b1 Mon Sep 17 00:00:00 2001 From: Ashley Willard Date: Thu, 2 Jan 2025 15:51:14 -0800 Subject: [PATCH 5/5] fix tests --- spec/packs_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/packs_spec.rb b/spec/packs_spec.rb index f31468e..a3d1060 100644 --- a/spec/packs_spec.rb +++ b/spec/packs_spec.rb @@ -339,12 +339,12 @@ def write_codeownership_config end context 'when the app has a README template' do - it 'uses the template to create the README_TODO.md' do + it 'uses the template to create the README.md' do write_file('README_TEMPLATE.md', 'This is the template') Packs.create_pack!(pack_name: 'packs/organisms') ParsePackwerk.bust_cache! - actual_readme_todo = ParsePackwerk.find('packs/organisms').directory.join('README_TODO.md') - expect(actual_readme_todo.read).to eq 'This is the template' + actual_readme = ParsePackwerk.find('packs/organisms').directory.join('README.md') + expect(actual_readme.read).to eq 'This is the template' end context 'and a custom path is specified for the README template' do @@ -354,12 +354,12 @@ def write_codeownership_config YML end - it 'uses the template to create the README_TODO.md' do + it 'uses the template to create the README.md' do write_file('my_folder/README_STUFF.md', 'This is the custom template') Packs.create_pack!(pack_name: 'packs/organisms') ParsePackwerk.bust_cache! - actual_readme_todo = ParsePackwerk.find('packs/organisms').directory.join('README_TODO.md') - expect(actual_readme_todo.read).to eq 'This is the custom template' + actual_readme = ParsePackwerk.find('packs/organisms').directory.join('README.md') + expect(actual_readme.read).to eq 'This is the custom template' end end end