Skip to content

Remove moment as a dependency for apollo-language-server#2595

Merged
trevor-scheer merged 1 commit intoapollographql:masterfrom
nickpith:remove_moment_from_language_server
Apr 18, 2022
Merged

Remove moment as a dependency for apollo-language-server#2595
trevor-scheer merged 1 commit intoapollographql:masterfrom
nickpith:remove_moment_from_language_server

Conversation

@nickpith
Copy link
Copy Markdown
Contributor

@nickpith nickpith commented Apr 14, 2022

There is really no strong need for moment to a dependency of apollo-language-server when it is used just to calculate durations in milliseconds. A simple constant will suffice instead.

Also added some tests to prove that this change works but is by no means exhaustive.

Related: #2323.

TODO:

  • Update CHANGELOG.md* with your change (include reference to issue & this PR)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

*Make sure changelog entries note which project(s) has been affected. See older entries for examples on what this looks like.

@apollo-cla
Copy link
Copy Markdown

@nickpith: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

There is really no strong need for `moment` to a dependency of `apollo-language-server` when it is used just to calculate durations in milliseconds. A simple constant will suffice instead.

Also added some tests to prove that this change works but is by no means exhaustive.

Resolves apollographql#2323.
@trevor-scheer
Copy link
Copy Markdown
Contributor

Thanks @nickpith! I updated your PR description so it doesn't autoclose the related issue. We still need to remove moment in the CLI itself in order to close that one out (even though the author of that issue wrote in that it's due to the usage in apollo-language-server).

As mentioned in #2593, I'll make a best effort w.r.t. this repo and try to get a release of this cut in a reasonable time, but my current focus is AS4. Apologies for any inconvenience this causes in the meantime.

@trevor-scheer trevor-scheer merged commit bfb68c2 into apollographql:master Apr 18, 2022
@nickpith
Copy link
Copy Markdown
Contributor Author

@trevor-scheer Thanks for merging and fixing the description. I can understand the focus on AS4 so whenever you can get to it would be fine!

@nickpith nickpith deleted the remove_moment_from_language_server branch April 18, 2022 17:35
trevor-scheer pushed a commit that referenced this pull request May 5, 2022
There is really no strong need for `moment` to a dependency of `apollo-language-server` when it is used just to calculate durations in milliseconds. A simple constant will suffice instead.

Also added some tests to prove that this change works but is by no means exhaustive.

Related: #2323.
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.

4 participants