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

Fixing restyled to ignore third_party #392

Merged
merged 14 commits into from
Apr 17, 2020
Merged

Fixing restyled to ignore third_party #392

merged 14 commits into from
Apr 17, 2020

Conversation

woody-apple
Copy link
Contributor

Problem

third_party is kicking restyled, need to fix that

Summary of Changes

Ongoing... testing

.restyled.yml Outdated Show resolved Hide resolved
@woody-apple
Copy link
Contributor Author

Not sure why the restyled check is counting here (build fails because branch is gone)...

@woody-apple
Copy link
Contributor Author

@hawk248 @anush-apple @jelderton ?

Copy link

@jelderton jelderton left a comment

Choose a reason for hiding this comment

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

Looks cleaner than before. Thanks

Comment on lines +27 to +29
- "third_party/**/*"
- "third_party/**"
- "third_party/*"
Copy link
Contributor

@rwalker-apple rwalker-apple Apr 16, 2020

Choose a reason for hiding this comment

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

this is really unfortunate. is restyled really this stupid?

Copy link
Contributor Author

@woody-apple woody-apple Apr 16, 2020

Choose a reason for hiding this comment

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

Not sure it is a multiple 'really' level of unfortunate, but it seems to work for now. Trying to pair it back down as I understand their syntax.

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed to single level really

- "third_party/**/*"
- "third_party/**"
- "third_party/*"
- "build/**/*"
Copy link
Contributor

Choose a reason for hiding this comment

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

we want restyled looking at build, as long as the files are part of the git repository

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"We want" can you explain? which we don't want?

Copy link
Contributor

@rwalker-apple rwalker-apple Apr 16, 2020

Choose a reason for hiding this comment

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

we want restyled to look at all the things in build/ that are in git

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 should be addressed separately. For now, we're should ignore build, we can bring this back in later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aside: Ideally, we should have a real build directory then, given it's a mix of built products, and scripts

- "third_party/**"
- "third_party/*"
- "build/**/*"
- "autom4te.cache/**/*"
Copy link
Contributor

Choose a reason for hiding this comment

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

there are no files in this directory in git...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, but it matters for locally restyled

Copy link
Contributor

@rwalker-apple rwalker-apple Apr 16, 2020

Choose a reason for hiding this comment

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

restyled can't look at .gitignore? I guess this is no biggie, but it means .gitignore and this file have to be kept in sync.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, feel free to open up an issue there.

corresponding to the number of months since January 1, 2020, and generally mean
the last Friday of that month. E.g. M1 corresponds to EOB January 24,

2020. The Morrow milestone is a special milestone that means "unprioritized" or
Copy link
Contributor

@rwalker-apple rwalker-apple Apr 16, 2020

Choose a reason for hiding this comment

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

this is a misfire, "2020." isn't a numbered list item.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's following the default .md formatter. Do we have a config anywhere we should use? We are not going to manually verify each thing the restyler does and accept/decline it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should probably just trick the thing by wrapping it another way

yes: I plan to verify everything the restyler does, lest we break our tree.

@woody-apple woody-apple merged commit 0c46b2e into project-chip:master Apr 17, 2020
Copy link
Contributor

@anush-apple anush-apple left a comment

Choose a reason for hiding this comment

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

Approved.

@woody-apple woody-apple deleted the fixing-restyled branch May 8, 2020 02:05
mkardous-silabs pushed a commit to mkardous-silabs/connectedhomeip that referenced this pull request Dec 19, 2022
Merge in WMN_TOOLS/matter from feature/wifi_gsdk_integration_slc_1.0 to silabs_slc_1.0

Squashed commit of the following:

commit 819dccc274991177cf95ea95ddf5219b9affe87a
Author: Rehan Rasool <[email protected]>
Date:   Thu Dec 15 15:26:50 2022 -0500

    Remove brd4187c from supported wi-fi boards as it does not compile

commit 9934ca51034e338aab1bcf7e2a031c4f98e8e090
Author: Rehan Rasool <[email protected]>
Date:   Thu Dec 15 14:31:20 2022 -0500

    Fix Wi-Fi stash name so when SQA stage is enabled, it works same way as it does for Ninja builds

commit 305a3d7e4a953e359c3102640651899d072f912f
Author: Rehan Rasool <[email protected]>
Date:   Thu Dec 15 14:28:21 2022 -0500

    Fix missed board compatibilities for wi-fi apps

... and 25 more commits
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.

6 participants