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

[cli] Provide better logs related to dir structure when running tests #4213

Merged
merged 2 commits into from Jan 4, 2022

Conversation

Brianzchen
Copy link
Member

In #4210 a contributor was unable to run tests when create a new lib def because they may have made a mistake when initially trying to create the dir structure. This throws an error but it's not one that's descriptive enough to help them pinpoint where to go to solve the issue.

Other notes, when I was building this fix I opened the definitions/npm dir in my finder and then it started throwing a lot of errors related to DS_Store I think instead of throwing we should just exit in that case so I've added that also.

@@ -168,6 +168,8 @@ export async function getLibDefs(defsDir: string): Promise<Array<LibDef>> {
const defsDirItems = await fs.readdir(defsDir);
await P.all(
defsDirItems.map(async item => {
if (item === '.DS_Store') return;
Copy link
Member

Choose a reason for hiding this comment

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

Could we instead get rid of the DS_Store files and add a test to make sure they don't come back?

Copy link
Member Author

Choose a reason for hiding this comment

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

How do we do that? Mine randomly appeared because I opened my finder, it's not like you can gitignore it because the issue is when we run it locally and because it creates a DS_Store in every single dir means that when cleaning up I was trying to go into every definitions/npm/@scope to delete this.

Unless you're saying when we run this loop we should delete it instead of returning? Not sure which approach is best here, I imagine deleting isn't good because DS_Store is used to retain folder positioning if someone does open the repo in their finder which has some usage.

Copy link
Member

Choose a reason for hiding this comment

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

Right yeah makes sense, I see how it can be an issue when running it locally. 👍🏼

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I can see how this code makes no sense when reading this later. I'll add a comment to describe why this issue needs to be exited before I merge

@Brianzchen Brianzchen merged commit 25f15b8 into flow-typed:master Jan 4, 2022
@Brianzchen Brianzchen deleted the unformatted-defs-logs branch January 4, 2022 10:17
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.

None yet

2 participants