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

[CI] Fix for jsdoc hook in pre-commit #92013

Merged
merged 1 commit into from
May 16, 2024

Conversation

patwork
Copy link
Contributor

@patwork patwork commented May 16, 2024

Info

This is a fix for jsdoc hook from #91597

Currently, the hooks in the pre-commit are only triggered if the specified files have been modified. In the case of the jsdoc hook, engine.js, config.js and features.js from the platform/web/js/engine directory are checked.

The problem is that jsdoc requires all these 3 files in the parameter line and not, as pre-commit does, only those that were modified in the commit.

When, for example, only feature.js is changed in a commit, an error will appear:

jsdoc....................................................................Failed
- hook id: jsdoc
- exit code: 1

[...]

Undefined symbol! Engine
/godot/platform/web/js/jsdoc2rst/publish.js:315
				throw new Error('Undefined symbol!');
				^

Error: Undefined symbol!

The solution is to permanently add the names of all 3 files to the argument line and disable the automatic addition of modified file names by pre-commit (pass_filenames: false).

        args:
          - --template
          - platform/web/js/jsdoc2rst/
          - platform/web/js/engine/engine.js
          - platform/web/js/engine/config.js
          - platform/web/js/engine/features.js
          - --destination
          - ''
          - -d
          - dry-run
        pass_filenames: false

@patwork patwork requested a review from a team as a code owner May 16, 2024 11:55
@AThousandShips AThousandShips added this to the 4.3 milestone May 16, 2024
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@Repiteo Repiteo left a comment

Choose a reason for hiding this comment

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

Only tested this in --all-files contexts, so never noticed this quirk; great catch!

@akien-mga akien-mga merged commit f4b047a into godotengine:master May 16, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

4 participants