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

Fix #1343 - add fsevents for MacOS users #1345

Merged
merged 1 commit into from Sep 25, 2023

Conversation

zulus
Copy link
Contributor

@zulus zulus commented Sep 25, 2023

Just add to package.json, with force will be installed even on linux/windows

@zulus
Copy link
Contributor Author

zulus commented Sep 25, 2023

/request-license-review

@zulus
Copy link
Contributor Author

zulus commented Sep 25, 2023

@mickaelistria am I doing something wrong? Looks like my comment with /request-license-review has been ignored (In theory I have rights)

@mickaelistria
Copy link
Contributor

I don't know. I also faced this issue earlier and I noticed that the GitHub action is marked as skipped ( https://github.com/eclipse-wildwebdeveloper/wildwebdeveloper/actions/runs/6300253210 ). I don't know what that means, but I know that's not expected.

@vrubezhny
Copy link
Contributor

vrubezhny commented Sep 25, 2023

@zulus After you commented with /request-license-review the following IP Team Review request was created: npm/npmjs/@babel/parser/7.23.0.
Then it takes some time for the request to be resolved (sometimes it takes days due to the need to be manually resolved).
After the review request is resolved as Approved the License check / npm-check build job is to be re-run manually - if no other NPM Modules require a review (as some modules might be updated on every build) then it should be built with success.

@zulus
Copy link
Contributor Author

zulus commented Sep 25, 2023

I don't know. I also faced this issue earlier and I noticed that the GitHub action is marked as skipped ( https://github.com/eclipse-wildwebdeveloper/wildwebdeveloper/actions/runs/6300253210 ). I don't know what that means, but I know that's not expected.

Based on workflow definition is ok. These jobs skip if PR was just commented with '/request-license-review'

@zulus After you commented with /request-license-review the following IP Team Review request was created: npm/npmjs/@babel/parser/7.23.0. Then it takes some time for the request to be resolved (sometimes it takes days due to the need to be manually resolved). After the review request is resolved as Approved the License check / npm-check build job is to be re-run manually - if no other NPM Modules require a review (as some modules might be updated on every build) then it should be built with success.

Thank You, previously I saw auto rocket emoji. Looks like I have to manually check gitlab and search for libraries

Copy link
Contributor

@vrubezhny vrubezhny left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@zulus Thanks for the improvement!

It'd be good to mention somehow this change in README or CONTRIBUTION - I don't know where exactly - why do we need this NPM Dependency to be included?
(Usually we do not keep not used dependencies with no reason, so someone, who doesn't know why this one is added, might suggest removing it. So it'd better to be documented somehow)

@zulus
Copy link
Contributor Author

zulus commented Sep 25, 2023

The IP Team Review requests are created regarding the proposal:

@vrubezhny
Copy link
Contributor

Thank You, previously I saw auto rocket emoji. Looks like I have to manually check gitlab and search for libraries

Sometimes it takes quite a long time for dependencies to be approved by IP Team, so we didn't ever think on adding some automation here.

You can skip visiting the https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/?sort=created_date&state=closed&author_username=wildwebdeveloper-bot&first_page_size=100 - Just re-run npm-check job in few minutes after you did /request-license-review - normally it takes 10-30 minutes for a dependency to be approved if no IP Team manual intervention is required depending on the workload of the IP Team's bot.

@zulus
Copy link
Contributor Author

zulus commented Sep 25, 2023

Looks good to me.

@zulus Thanks for the improvement!

It'd be good to mention somehow this change in README or CONTRIBUTION - I don't know where exactly - why do we need this NPM Dependency to be included? (Usually we do not keep not used dependencies with no reason, so someone, who doesn't know why this one is added, might suggest removing it. So it'd better to be documented somehow)

this is not relared to fsevents, error win npm I also saw in my other PR. One of the packages have relaxed dependencies. I'll check which one

@mickaelistria
Copy link
Contributor

@vrubezhny I think something regressed in the license-check. It used to react and mention the opened IP reviews if any. I don't see this now...

@vrubezhny
Copy link
Contributor

vrubezhny commented Sep 25, 2023

@vrubezhny I think something regressed in the license-check. It used to react and mention the opened IP reviews if any. I don't see this now...

Some time ago I reported an issue to license check - it was creating a duplicating review requests for the same NPM dependency after the first one is already created and continuing repeatedly run the check with -review argument - probably it is fixed now and the repeated executions of /request-license-review get canceled or something like this.

UPD: Or not fixed, but just closed: eclipse/dash-licenses#202

@zulus
Copy link
Contributor Author

zulus commented Sep 25, 2023

It'd be good to mention somehow this change in README or CONTRIBUTION - I don't know where exactly - why do we need this NPM Dependency to be included? (Usually we do not keep not used dependencies with no reason, so someone, who doesn't know why this one is added, might suggest removing it. So it'd better to be documented somehow)

@vue/compiler-core (one of vue ls deps) have dependency to "@babel/parser" : "^7.21.3" this is why license warning appear, they recently published new version

@zulus
Copy link
Contributor Author

zulus commented Sep 25, 2023

Looks like ready to merge ;)

@zulus zulus merged commit 30e19f9 into eclipse-wildwebdeveloper:master Sep 25, 2023
6 checks passed
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