Skip to content

Remove unused NPM modules#4151

Merged
ehelms merged 2 commits intotheforeman:rpm/developfrom
ehelms:update-npm-modules
Oct 2, 2019
Merged

Remove unused NPM modules#4151
ehelms merged 2 commits intotheforeman:rpm/developfrom
ehelms:update-npm-modules

Conversation

@ehelms
Copy link
Member

@ehelms ehelms commented Sep 30, 2019

This is a large PR as there are a large number of nodejs modules. This updates just the Foreman modules for now to a spec that can support building for the SCL. These changes come with some updated bundle tarballs, and updated dependencies for those bundles. This also removes two packages that were not required by any existing spec: nodejs-select2 and nodejs-jquery-flot. I didn't feel like making 150+ PRs for each package. So my hope is if this succeeds we feel confident enough to merge and fix up any issues after.

@ehelms
Copy link
Member Author

ehelms commented Sep 30, 2019

Also, these updates are based on theforeman/npm2rpm#58

@ehelms ehelms force-pushed the update-npm-modules branch 2 times, most recently from aaab038 to 571f9a7 Compare September 30, 2019 20:53
@ehelms
Copy link
Member Author

ehelms commented Sep 30, 2019

This now only represents about half the modules in an attempt to get it through CI.

@ehelms ehelms force-pushed the update-npm-modules branch 2 times, most recently from cd86ca2 to 325693a Compare October 1, 2019 01:16
@ehelms
Copy link
Member Author

ehelms commented Oct 1, 2019

[test rpm]

@ehelms ehelms force-pushed the update-npm-modules branch from 325693a to ebc8ce6 Compare October 1, 2019 13:43
@ehelms
Copy link
Member Author

ehelms commented Oct 1, 2019

[test rpm]

@ehelms ehelms force-pushed the update-npm-modules branch from ebc8ce6 to 280f1eb Compare October 2, 2019 00:24
@ehelms ehelms changed the title Update npm modules Remove unused NPM modules Oct 2, 2019
@ehelms
Copy link
Member Author

ehelms commented Oct 2, 2019

This PR is now focused on removing unused NPM modules. This took a tactical approach to identify unused modules by analyzing Requires within all spec files. The scripts I used:

#!/usr/bin/env ruby
  
BASE_DIR = "packages/foreman/"

dirs = Dir.glob("#{BASE_DIR}nodejs-*/")

dirs = dirs.collect do |package|
  `basename #{package}`
end

dirs = dirs.select do |package|
  npm_module = "npm\\(#{package.gsub('nodejs-', '').rstrip()}\\)"
  found = `grep -r #{npm_module} packages/`
  found = found.split("\n")
  found = found.select { |found_pkg| !found_pkg.include?('bundled') }

  npm_module = "npm\\(@#{package.gsub('nodejs-', '').rstrip().split("-").first}"
  found_grouped = `grep -r #{npm_module} packages/`
  found_grouped = found_grouped.split("\n")

  found = found + found_grouped

  if found.empty?
    true
  else
    false
  end
end

dirs.sort.each do |package|
  puts package
  `git rm -rf #{BASE_DIR}/#{package}`
end

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I had considered storing the strategy somewhere. Is there already code that uses this?

strategy: bundle
nodejs-babel-plugin-lodash:
strategy: bundle
#nodejs-babel-plugin-syntax-dynamic-import: {}
Copy link
Member

Choose a reason for hiding this comment

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

Nit: should this be removed?

@ehelms ehelms force-pushed the update-npm-modules branch from 280f1eb to a4e4dab Compare October 2, 2019 12:00
@ehelms
Copy link
Member Author

ehelms commented Oct 2, 2019

The work I started in Obal to add an update-spec command (theforeman/obal#207) uses the strategy and name as input.

@ehelms ehelms merged commit d96c98d into theforeman:rpm/develop Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants