Skip to content

[AT-65] Added warning when no files found in fileset#1007

Merged
JakeDawkins merged 5 commits intomasterfrom
jake/fileset-warning
Feb 15, 2019
Merged

[AT-65] Added warning when no files found in fileset#1007
JakeDawkins merged 5 commits intomasterfrom
jake/fileset-warning

Conversation

@JakeDawkins
Copy link
Copy Markdown
Contributor

@JakeDawkins JakeDawkins commented Feb 11, 2019

Fixes #1002
Fixes #702
Fixes #883

File count warnings

Whenever the VS Code extension opens a file, it tries to find a projectForFile. If the includes or excludes field in the Apollo Config file is misconfigured, it may never actually find the project for a given file.

The way the status bar/stats reporting command is setup, if you click the icon to report stats while having one of those files open, it will not find the associated project and warn that the project may be misconfigured. Which is okay, but I think we can do better.

On project creation, this PR checks the FileSet to see how many files are included. If there are 0 files in the FileSet (excluding .env and configs), we issue a warning but not an error. that's because there's no way to know that there are supposed to be files there. Maybe the user is bootstrapping a new project and there just aren't any files yet.

Includes/Excludes fixes

Because of how we were using glob/minimatch on the includes/excludes, there were some issues:

  1. includes/excludes that were relative (i.e. ./src/__tests__) would not work with FileSet.includesFile. To fix, includesFile now uses the results of allFiles ([apollo-graphql] vscode gql tag functionality broken when relative globs used in includes config #1002)
  2. excludes wasn't properly being filtered out in FileSet.allFiles, and wouldn't allow proper filtering if the glob was relative (the same issue that originally was in includesFile

TODO:

  • Update CHANGELOG.md* with your change (include reference to issue & this PR)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

*Make sure changelog entries note which project(s) has been affected. See older entries for examples on what this looks like.

screen shot 2019-02-11 at 5 11 07 pm

screen shot 2019-02-11 at 5 13 05 pm

Copy link
Copy Markdown
Contributor

@trevor-scheer trevor-scheer left a comment

Choose a reason for hiding this comment

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

This is a nice win for usability, and straightforward. Nice work!

I'll leave it up to you, but it would be awesome to have a few tests for FileSet, specifically around includes/excludes behavior.

Comment thread packages/apollo-language-server/src/fileSet.ts Outdated
Comment thread packages/apollo-language-server/src/fileSet.ts Outdated
Comment thread packages/apollo-language-server/src/fileSet.ts Outdated
Comment thread packages/apollo-language-server/src/fileSet.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants