Skip to content

Investigate config module dependencies and clean up legacy root-controller usage#464

Open
sheshankarvapally wants to merge 4 commits into
thoth-tech:10.0.xfrom
sheshankarvapally:investigate-config-dependencies-10x-clean
Open

Investigate config module dependencies and clean up legacy root-controller usage#464
sheshankarvapally wants to merge 4 commits into
thoth-tech:10.0.xfrom
sheshankarvapally:investigate-config-dependencies-10x-clean

Conversation

@sheshankarvapally
Copy link
Copy Markdown

@sheshankarvapally sheshankarvapally commented Apr 22, 2026

Summary:

This PR continues Task 2 by migrating the legacy AngularJS config dependency area away from CoffeeScript as part of the Angular 17 migration effort.

Changes made:

  • Removed config.coffee

  • Removed root-controller.coffee because AppCtrl had no functional logic

  • Removed routing.coffee

  • Removed runtime.coffee

  • Removed dependency on the old doubtfire.config module

  • Updated index.html to remove AppCtrl usage

  • Kept required config submodules such as:

    • doubtfire.config.vendor-dependencies
    • doubtfire.config.analytics
    • doubtfire.config.debug
    • doubtfire.config.privacy-policy

Investigation Findings:

  • The removed config layer was acting mostly as a legacy wrapper
  • AppCtrl/root-controller had no active business logic
  • The AngularJS config dependency could be simplified safely
  • Legacy CoffeeScript configuration files can now be migrated cleanly

Testing:

  • Confirmed AppCtrl references are removed
  • Confirmed old doubtfire.config dependency is removed
  • Confirmed Angular bundle generation completes successfully
  • Verified application loads after migration cleanup
  • Backend/API issues observed were local Docker setup issues and unrelated to this migration

Copy link
Copy Markdown

@WaelAlahamdi WaelAlahamdi left a comment

Choose a reason for hiding this comment

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

Hi @sheshankarvapally,
Reviewed and tested locally.
The application loads correctly, navigation works as expected, and no console/runtime errors were encountered during testing.
The migration cleanup and legacy CoffeeScript dependency removal align with the task objectives. Looks good to me.

Copy link
Copy Markdown

@Sujay-Deakin Sujay-Deakin left a comment

Choose a reason for hiding this comment

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

Tested this locally, good to see the bundle still generates cleanly with those four CoffeeScript config files removed. The cleanup is well-scoped, and the wrapper being stripped out is a nice step forward in the migration. Looks good to me.

Copy link
Copy Markdown

@sakethsram8888 sakethsram8888 left a comment

Choose a reason for hiding this comment

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

Tested this PR on my local machine and everything worked as expected. The migration cleanup looks solid, and I didn’t encounter any issues related to the removed config layer or AppCtrl changes. The application loaded successfully and the bundle generation completed without errors.

I also appreciate the investigation and simplification of the legacy AngularJS config dependency area ,the changes make the migration path much cleaner and easier to maintain.

Copy link
Copy Markdown

@millyamolo millyamolo left a comment

Choose a reason for hiding this comment

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

Tested locally on the PR, App loads correctly after login and there are no browser console errors related to AppCtrl, config module dependencies, or routing. Navigation appears to work as expected. Happy to approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants