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

[Storage] Fix rollup warnings #3993

Closed
HarshaNalluru opened this issue Jun 21, 2019 · 21 comments
Closed

[Storage] Fix rollup warnings #3993

HarshaNalluru opened this issue Jun 21, 2019 · 21 comments
Labels
Client This issue points to a problem in the data-plane of the library. help wanted This issue is tracking work for which community contributions would be welcomed and appreciated test-utils-recorder Label for the issues related to the common recorder

Comments

@HarshaNalluru
Copy link
Member

No description provided.

@HarshaNalluru HarshaNalluru added Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files) labels Jun 21, 2019
@HarshaNalluru
Copy link
Member Author

HarshaNalluru commented Jul 5, 2019

Fixes some warnings - #4074

@HarshaNalluru
Copy link
Member Author

Warnings
image

@ramya-rao-a
Copy link
Contributor

Is this still applicable now that we are removing the circular dependencies by moving the code to the same file?

@HarshaNalluru
Copy link
Member Author

Regarding eval warning -
It's unrelated to storage and should be tracked under the common recorder.

Nevertheless, logged an issue in the sinonjs/nise repo regarding eval.
sinonjs/nise#110

@HarshaNalluru
Copy link
Member Author

Current warnings -
image

Circular references are being fixed at #5993 and #5998

@HarshaNalluru HarshaNalluru added the test-utils-recorder Label for the issues related to the common recorder label Nov 5, 2019
@HarshaNalluru HarshaNalluru self-assigned this Nov 5, 2019
@ramya-rao-a ramya-rao-a assigned 1hw7 and willmtemple and unassigned 1hw7 Nov 17, 2019
@ramya-rao-a
Copy link
Contributor

@willmtemple Now that the PRs for fixing the circular references are merged, can you update this issue with a screenshot of remaining warnings from rollup?

@ramya-rao-a
Copy link
Contributor

Moving this issue to the Jan milestone to follow up on leftover warnings if any

@ramya-rao-a ramya-rao-a added this to the [2020] January milestone Nov 19, 2019
@jeremymeng jeremymeng self-assigned this Dec 2, 2019
@jeremymeng
Copy link
Member

Current warnings (test only)

image

@jeremymeng
Copy link
Member

Adding fs to output.globals gets rid of the last warning, but not a proper fix.

@jeremymeng jeremymeng removed this from the [2020] January milestone Jan 7, 2020
@HarshaNalluru
Copy link
Member Author

Thanks!!!

@HarshaNalluru HarshaNalluru removed their assignment Apr 10, 2020
@ramya-rao-a
Copy link
Contributor

I don't see any more rollup warnings when running rushx build for

  • storage-blob
  • storage-queue
  • storage-file-share
  • storage-file-datalake

@jeremymeng, @HarshaNalluru Can we close this?

@jeremymeng
Copy link
Member

The warnings are from build:test script

@willmtemple
Copy link
Contributor

The remaining warnings that we are seeing here in test are some isolated warnings about dependencies not matching that will be solved by rollup config abstraction work and these warnings about the use of eval in the Nise package.

Marking this up for grabs if anyone wants to take a stab at the Nise issue, since it needs a little investigation. We can probably handle this warning by silencing it in the test configurations.

@HarshaNalluru
Copy link
Member Author

HarshaNalluru commented Aug 27, 2020

I had an issue logged in the nise repo sinonjs/nise#110 long back, which has been moved to sinonjs/fake-timers#319 and seems like it is actively being worked on.

We can disable the warnings through the rollup config.

Regarding "fs-extra", I have a PR to remove it completely from the repo #9402
Somehow, the keyvault-certificates CI failed and blocked the PR. I'll revive the PR back and take a stab at it again!

@willmtemple
Copy link
Contributor

@HarshaNalluru I did end up including an inhibitor for the Nise/Sinon eval warning in the proposed shared rollup configuration. #10923

@ramya-rao-a ramya-rao-a added help wanted This issue is tracking work for which community contributions would be welcomed and appreciated and removed Up for grabs labels Sep 15, 2020
@ramya-rao-a
Copy link
Contributor

@willmtemple, Is there any other pending work here to be done other than using the shared rollup config once it is available?

@willmtemple
Copy link
Contributor

@ramya-rao-a In theory, it is the same as using the shared rollup config. In practice, adapting to it may require some code changes if storage's code is not compatible with the shared rollup config out-of-the-box. It will depend a little bit once I can dig into it.

Copy link

Hi @HarshaNalluru, we deeply appreciate your input into this project. Regrettably, this issue has remained inactive for over 2 years, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 25, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 25, 2024
@xirzec xirzec removed this from the Backlog milestone May 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. help wanted This issue is tracking work for which community contributions would be welcomed and appreciated test-utils-recorder Label for the issues related to the common recorder
Projects
None yet
Development

No branches or pull requests

7 participants