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

refactor: use map for declarations and name suggestions #4410

Merged
merged 4 commits into from Feb 20, 2022

Conversation

dnalborczyk
Copy link
Contributor

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:

Description

@codecov
Copy link

codecov bot commented Feb 19, 2022

Codecov Report

Merging #4410 (35e5e1b) into master (fa4e1b7) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4410      +/-   ##
==========================================
- Coverage   98.73%   98.72%   -0.01%     
==========================================
  Files         204      204              
  Lines        7323     7322       -1     
  Branches     2081     2080       -1     
==========================================
- Hits         7230     7229       -1     
  Misses         34       34              
  Partials       59       59              
Impacted Files Coverage Δ
src/ExternalModule.ts 100.00% <100.00%> (ø)
src/Module.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa4e1b7...35e5e1b. Read the comment docs.

@dnalborczyk
Copy link
Contributor Author

dnalborczyk commented Feb 19, 2022

@lukastaegert this will initialize always with 1, instead of (index-based) 0. is that an oversight, or done on purpose?

if (!this.nameSuggestions[name]) this.nameSuggestions[name] = 0;
this.nameSuggestions[name] += 1;

I left the functionality "as is", but I can change it to be 0-based.

@guybedford
Copy link
Contributor

I believe the 1-based numbering is intentional for human readability, although @lukastaegert may be able to correct me on this.

@lukastaegert
Copy link
Member

this will initialize always with 1

I think this logic is ancient and predates even me but yes, readability was probably the idea. I do not think it is super important, though.

@lukastaegert lukastaegert merged commit 213e721 into rollup:master Feb 20, 2022
@dnalborczyk dnalborczyk deleted the refactor-maps branch February 22, 2022 01:24
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

3 participants