Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

Conversation

@icecream17
Copy link
Contributor

I can't figure out why marked is needed. It seems like it's never used

@icecream17
Copy link
Contributor Author

icecream17 commented Jul 11, 2021

Ok, I see that it is imported once in spec, but the import is also unused.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jul 14, 2021

marked was added in this commit (not part of a PR): 61651db and removed in #12903 (specifically here: diff, commit).

(I got this from trawling through the git blame (for example, here) of package.json and spec/atom-reporter.coffee...)

If I understand correctly, it was used to render some logging output as fancy markdown, which I presume was only seen while running the tests. But that's from just a quick look, so I might be wrong. In any case, all use of marked directly in Atom core (not as a sub-dependency of another package) does seem to have been removed.

@icecream17
Copy link
Contributor Author

Hmm, settings-view says ^1.2.0 but I wonder if 1.2.9 actually breaks settings-view somehow

atom/package-lock.json

Lines 7070 to 7081 in 0ede4a6

"settings-view": {
"version": "https://www.atom.io/api/packages/settings-view/versions/0.261.8/tarball",
"integrity": "sha512-Zk7Cd2YRcL0inBbOg+vfIG5C4W1mrtY6RR/bvJ0Ddx4FCswxndRndVn575RbAJN5uvbt5hoQdvXmEdd0ksdlVA==",
"requires": {
"async": "^3.2.0",
"dompurify": "^1.0.2",
"etch": "0.9.0",
"fs-plus": "^3.0.0",
"fuzzaldrin": "^2.1",
"glob": "4.3.1",
"hosted-git-info": "^2.1.4",
"marked": "^1.2.0",

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jul 15, 2021

Yeah, I just did some CI runs to test that, looks like it to me.

❌ This PR: https://dev.azure.com/DeeDeeG/b/_build/results?buildId=1189&view=results
✔️ This PR, but revert package-lock.json changes https://dev.azure.com/DeeDeeG/b/_build/results?buildId=1188&view=results

Given these results, I think reverting the change in package-lock.json should do it.

(Side note: I don't really understand how package.json can be updated by this PR and have no effect on package-lock.json, but it looks like that's correct somehow. Deduping is weird I guess? I ran npm install and apm install and both thought package-lock.json needed no changes, despite deleting marked as a top-level dependency from package.json.)

@icecream17 icecream17 marked this pull request as ready for review July 15, 2021 19:10
@sadick254 sadick254 merged commit f9f0aa3 into atom:master Jul 19, 2021
@icecream17 icecream17 deleted the try-removing-marked branch July 23, 2021 17:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants