Add/handle namespaces - Githubissues

pzavolinsky / ts-unused-exports

ts-unused-exports finds unused exported symbols in your Typescript project

MIT License

749 stars 49 forks source link

Add/handle namespaces #96

Closed mrseanryan closed 4 years ago

mrseanryan commented 4 years ago

Find exports from within namespaces. If a namespace is imported, then scan for uses of the types from that namespace (to avoid false positives).

For performance, this feature is disabled by default. To enable, use the option --searchNamespaces.

Fixes #86

mrseanryan commented 4 years ago

hi @pzavolinsky this is the new PR - set up to point to master.

So it can be merged (commits will be added to master branch).

pzavolinsky commented 4 years ago

Oops, my bad, didn't check the PRs base branch before merging it.

@mrseanryan could you squash the commits in this PR? Let me know when it's done and I'll merge it.

mrseanryan commented 4 years ago

@pzavolinsky have squashed down the commits (less than 33% of what it was :) )

mrseanryan commented 4 years ago

hi Patricio, question - have you any thoughts about setting up a shared npm account ? I'm thinking that it could save us both some time (for example for branches like the add-namespace support). Of course, you could still get to review/approve PRs before they are merged to master. RegardsSean

___ Sean Ryan

On Monday, 30 December 2019, 12:28:03 CET, Patricio Zavolinsky <notifications@github.com> wrote:

Oops, my bad, didn't check the PRs base branch before merging it.

@mrseanryan could you squash the commits in this PR? Let me know when it's done and I'll merge it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

pzavolinsky commented 4 years ago

have you any thoughts about setting up a shared npm account ?

I'll look into it.

In the meantime, in scenarios like this one (where everything is OK, and only housekeeping remains), you could directly merge to master, update the changelog, bump the version and just let me know (in a separate issue), that master is ready to publish.

On a related note, I think we should try to squash commits down to a single commit per PR and rebase that commit after master (I'll added this last bit as a GitHub rule). A single commit per PR gives us two advantages:

In case of a rollback or a bisect, is much easier to find the PR where the issue appeared.
All the commits in master are working, valid commits (i.e. there are not intermediate, potentially broken commits in master).

Cheers!

mrseanryan commented 4 years ago

Thanks @pzavolinsky

I see - it could have been merged by me so, but that was not clear to me at the time. I'd still be interested in a shared npm account if it's possible...

RE squash commits - Do you mind if I add a note about that to the Contrib.md? Else ill completely forget.

About base on master - sometimes I base a PR on another PR, until the first is merged - for example if 1st PR has refactorings or new ts version etc, its nice to pick them up. Else can kind of get blocked...

Could a github rule prevent merging 2nd PR into the 1st one? (so only allow merging to master)

Regards Sean

pzavolinsky commented 4 years ago

Do you mind if I add a note about that to the Contrib.md? Else ill completely forget.

Sure, go ahead.

Could a github rule prevent merging 2nd PR into the 1st one?

Not sure, I have to look into it.