Skip to content

Refactor common parts of convertConfig and convertEditorConfig into runFileConversion helper #274

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

Closed
MrCube42 opened this issue Nov 30, 2019 · 1 comment
Labels
status: aged away Not enough activity occurred on this issue in a reasonable amount of time to justify taking action. type: cleanup Code smells, incorrect tests, build systems, or other internal shenanigans

Comments

@MrCube42
Copy link
Contributor

This is a follow-up issue of this PR: #250.
It is only valid if this branch gets merged.

Original discussion #250 (comment):

Similarly, convertConfig and convertEditorConfig are really similar... I wonder if the common parts from both should be moved to some common runFileConversion helper? 🤷‍♂ maybe better to file a followup ticket to investigate; that feels like a kind of long-winded architectural debate.

🚀 Feature Request

This refactoring should not touch the general functionality. The goal is to remove a good portion of copy-paste code. However it may involve a greater architectural change in order to handle rule conversions and editor settings conversions similar.

Existing Behavior

  • convertConfig.ts converts TSLint rules into ESLint rules
  • convertEditorConfig.ts converts TSLint editor configuration settings into ESLint editor configuration settings
  • A lot of copy-paste code with little variation

Change Proposal

The potential runFileConversion could extract the commonalities out of the convertConfig and convertEditorConfig. Mainly these steps are taken:

  1. Existing configurations are read
  2. TSLint rules or settings are converted into their ESLint configurations
  3. Additonal steps (e.g. ESLint configurations are simplified based on extended ESLint presets)
  4. The simplified configuration is written to the output config file
  5. A summary of the results is printed to the user's console

The challenge is that the things to be done are quite similar, the data on which is operated differs in it's structure, e.g. the configuration that is read is of very different structures (rules files are typed very narrow, editor configuration settings are more unstructured)

@JoshuaKGoldberg JoshuaKGoldberg added the status: blocked We can't make progress on this issue until something else is resolved... label Dec 8, 2019
@JoshuaKGoldberg JoshuaKGoldberg added status: accepting prs Please, send in a PR to resolve this! ✨ type: cleanup Code smells, incorrect tests, build systems, or other internal shenanigans and removed status: blocked We can't make progress on this issue until something else is resolved... labels Dec 17, 2019
@JoshuaKGoldberg
Copy link
Member

In retrospect, I'm glad we didn't take action immediately to consolidate these functions. Their implementations have diverged a good bit in code and I don't think there's enough similarity between them to justify this issue anymore.

Yay, disappearing tech debt!

@JoshuaKGoldberg JoshuaKGoldberg added status: aged away Not enough activity occurred on this issue in a reasonable amount of time to justify taking action. and removed status: accepting prs Please, send in a PR to resolve this! ✨ labels Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: aged away Not enough activity occurred on this issue in a reasonable amount of time to justify taking action. type: cleanup Code smells, incorrect tests, build systems, or other internal shenanigans
Projects
None yet
Development

No branches or pull requests

2 participants