Skip to content
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

Add update_core_deprecations script #23

Merged
merged 4 commits into from
Jul 24, 2024
Merged

Conversation

tyb-talks
Copy link
Contributor

@tyb-talks tyb-talks commented Jul 17, 2024

This will update the deprecation ids for core in our reference yml file upon running:

node scripts/update_discourse_core_deprecations.mjs $DISCOURSE

where $DISCOURSE is the file path to your core discourse repository.

It also writes the paths of files where we call deprecated but no IDs are scraped from that same file into files_to_debug.txt.

Testing

Generated PR (no files to debug):
tyb-talks#8

Generated PR (with files to debug):
tyb-talks#9

Considerations

  1. The current approach in this PR is limited to finding IDs from deprecated calls within the same file, and not across different files (which would be the case if we exported functions that wrap around deprecated that are used in other files). This is sufficient for now.
  2. Adding an empty files_to_debug.txt file is not ideal but was required to ensure that even if no IDs needed to be updated, we still raise a PR whenever there are files to debug since the create-pull-request action relies on repo diffs to trigger.
  3. js-yaml is used instead of yaml primarily because of the noArrayIndent option which maintains the same indentation of YAML content as the ruby yaml package.
  4. I tried using some babel plugins for the parser to process gjs files directly into ASTs to no avail before realising that it would be better to use a separate preprocessing step with content-tag first (Extract preprocessor and babel plugin into their own packages ember-cli/ember-template-imports#143 (comment) clued me in on this).

@tyb-talks tyb-talks force-pushed the dev-update-core-deprecations branch from 7810886 to b074e25 Compare July 22, 2024 12:30
@tyb-talks tyb-talks marked this pull request as ready for review July 22, 2024 12:32

traverse(ast, {
CallExpression(path) {
if (t.isIdentifier(path.node.callee, { name: "deprecated" })) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the key point where the traverse visitor function finds the function call for deprecated in the current file's AST.

@davidtaylorhq
Copy link
Member

Can we still add manual deprecation IDs? (e.g. for deprecation IDs which are dynamically generated, and will not be picked up by this script)

I wonder if we need to split stuff into two files: one for the auto-detected deprecations from this script, and the other file can have manually-added ones?

@tyb-talks
Copy link
Contributor Author

tyb-talks commented Jul 23, 2024

Can we still add manual deprecation IDs? (e.g. for deprecation IDs which are dynamically generated, and will not be picked up by this script)
I wonder if we need to split stuff into two files: one for the auto-detected deprecations from this script, and the other file can have manually-added ones?

For this, we can use the old way of declaring such IDs explicitly in list.rb (before we moved to reading from file in https://github.com/discourse/discourse-deprecation-collector/pull/19/files) - we could add a second .concat call to https://github.com/discourse/discourse-deprecation-collector/blob/main/lib/deprecation_collector/list.rb#L12, and an array of such IDs.

@davidtaylorhq do you have any existing examples for dynamically generated deprecation IDs?

@tyb-talks tyb-talks requested a review from davidtaylorhq July 23, 2024 03:56
@davidtaylorhq
Copy link
Member

do you have any existing examples for dynamically generated deprecation IDs?

Only one I can think of at the moment is the deprecated plugin outlet arg (https://github.com/discourse/discourse/blob/23aa88d2038260481900478a3dcea0f283182984/app/assets/javascripts/discourse/app/components/header/contents.gjs#L103C17-L112C13)

@tyb-talks tyb-talks merged commit 0309869 into main Jul 24, 2024
4 checks passed
@tyb-talks tyb-talks deleted the dev-update-core-deprecations branch July 24, 2024 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants