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

Have tidy ensure that the there are no extra files in the webidls folder #13427

Closed
Manishearth opened this issue Sep 26, 2016 · 9 comments
Closed

Comments

@Manishearth
Copy link
Member

@Manishearth Manishearth commented Sep 26, 2016

We should probably lint that components/script/dom/webidls contains only *.webidl files.

Code: https://github.com/servo/servo/blob/master/python/tidy/servo_tidy/tidy.py

@highfive
Copy link

@highfive highfive commented Sep 26, 2016

@highfive
Copy link

@highfive highfive commented Sep 26, 2016

Please make a comment here if you intend to work on this issue. Thank you!

@gterzian
Copy link
Member

@gterzian gterzian commented Sep 26, 2016

@wafflespeanut
Copy link
Member

@wafflespeanut wafflespeanut commented Sep 26, 2016

@gterzian No, that checks the contents of the webidl files. We probably need a new function for doing that check.

@gterzian
Copy link
Member

@gterzian gterzian commented Sep 26, 2016

@wafflespeanut ok thanks for clarifying. Allright I'll give this one a try

@wafflespeanut
Copy link
Member

@wafflespeanut wafflespeanut commented Sep 26, 2016

Glad to hear :)

@gterzian
Copy link
Member

@gterzian gterzian commented Sep 26, 2016

@wafflespeanut @Manishearth

A question:

do we want to check for any type of 'unwanted' file inside components/script/dom/webidls or limit ourselves to the subset of FILE_PATTERNS_TO_CHECK see https://github.com/servo/servo/blob/master/python/tidy/servo_tidy/tidy.py#L44

I'm mainly asking because if we limit ourselves to FILE_PATTERNS_TO_CHECK, I can just plug into the existing 'file checking' generator pipeline from https://github.com/servo/servo/blob/master/python/tidy/servo_tidy/tidy.py#L834, for example by checking that every file ending in *.webidl is in the right folder, otherwise I think I'll have to do a completely separate check, which I guess would mean a separate run over every file in that folder.

Personally I'de like to just add a step to the existing generator pipeline...

@Manishearth
Copy link
Member Author

@Manishearth Manishearth commented Sep 26, 2016

@gterzian We want to ensure that this folder has only *.webidl files. webidl files elsewhere in the tree are okay (usually meaningless). Since the webidl files in that folder are read by the bindings generator, any other kind of file there is probably a mistake.

This is a different kind of check from the rest, let it stay completely separate.

@gterzian
Copy link
Member

@gterzian gterzian commented Sep 26, 2016

@Manishearth ok, thanks for the extra background info, that's all I need to get started :)

so I'll just add another step, akin to the config error check, that is unrelated to the generator pipeline that checks all files individually.

bors-servo added a commit that referenced this issue Oct 1, 2016
…peanut

Have tidy ensure that the there are no extra files in the webidls folder #13427

<!-- Please describe your changes on the following line: -->
 Have tidy ensure that the there are no extra files in the webidls folder #13427

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #13427 (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13447)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Oct 2, 2016
…peanut

Have tidy ensure that the there are no extra files in the webidls folder #13427

<!-- Please describe your changes on the following line: -->
 Have tidy ensure that the there are no extra files in the webidls folder #13427

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #13427 (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13447)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Oct 2, 2016
…peanut

Have tidy ensure that the there are no extra files in the webidls folder #13427

<!-- Please describe your changes on the following line: -->
 Have tidy ensure that the there are no extra files in the webidls folder #13427

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #13427 (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13447)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Oct 2, 2016
…peanut

Have tidy ensure that the there are no extra files in the webidls folder #13427

<!-- Please describe your changes on the following line: -->
 Have tidy ensure that the there are no extra files in the webidls folder #13427

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #13427 (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13447)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.