Skip to content

Conversation

aeisenberg
Copy link
Contributor

Previously, we were always using ** in the include path. the
effect of this was to always count lines in the entire
repository unless explicitly added to the paths-ignore. This
was incorrect behaviour. Now we only using ** if the include
path is otherwise empty.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.

Copy link
Contributor

@adityasharad adityasharad left a comment

Choose a reason for hiding this comment

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

Looks reasonable, look out for conflicts with the other LOC PR.

Comment on lines +71 to +72
// note that "**" is always included. The includes are for extra
// directories outside the normal structure.
Copy link
Contributor

Choose a reason for hiding this comment

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

Outdated comment?


t.deepEqual(results, {
js: 15,
js: 12,
Copy link
Contributor

Choose a reason for hiding this comment

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

Might conflict with your other PR.

Previously, we were always using `**` in the include path. the
effect of this was to always count lines in the entire
repository unless explicitly added to the paths-ignore. This
was incorrect behaviour. Now we only using `**` if the include
path is otherwise empty.
@aeisenberg aeisenberg force-pushed the aeisenberg/paths-include branch from 96b869f to 8e61fc2 Compare May 13, 2021 19:05
@aeisenberg aeisenberg merged commit 6a0bd27 into main May 13, 2021
@aeisenberg aeisenberg deleted the aeisenberg/paths-include branch May 13, 2021 19:25
@github-actions github-actions bot mentioned this pull request May 17, 2021
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.

2 participants