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

Use Electron distributon URL instead of atom #124

Merged
merged 1 commit into from
Jan 18, 2023
Merged

Use Electron distributon URL instead of atom #124

merged 1 commit into from
Jan 18, 2023

Conversation

jonian
Copy link

@jonian jonian commented Jan 10, 2023

Building atom from source fails because atom.io is down. Fixes atom-community/atom#486.

Building atom from source fails because atom.io is down. Fixes atom-community/atom#486.
@DeeDeeG
Copy link
Member

DeeDeeG commented Jan 16, 2023

I could merge this here, and I could tag a new release of apm in this repo, but I can't personally push a new version to the npm package registry at the moment.

So, to get the changes into atom-community/atom, this repo might have to be required there as a git URL (or, similarly, as a github tarball URL) pointing to this repo, instead of as an npm package.

An aside about getting this repo into atom-community/atom via a GitHub URL (click to expand):

Simply including this repo as a github URL is not ideal, as the .npmignore file excludes several unwanted files, but .npmignore would have no effect if getting the files off of a github URL. (The spec/fixtures dir has a 45MB tarball of the entire NodeJS source! We don't want that file especially.) But a branch or tag here could be published with just the wanted files by running npm pack and then deleting the repo contents and replacing them with only the files included in the generated tarball. Which respects .npmignore.)


I also don't have write access to the atom-community/atom repo, and I don't know if there is anyone interested in getting updated apm merged over there?

So, I think that's where this PR stands. If it is clear it can get into atom-community/atom, then I would be willing to do my part to merge this PR here and prepare this repo for including it over at atom-community/atom without going through the npm registry. Or if someone wants to (and can) publish to npm, that would also be fine, of course. I personally cannot make all of this happen, so someone with more access to atom-community/atom would need to be in the loop on this.

@icecream17
Copy link

Looks like I have the ability to merge at atom-community/atom (but not atom-community/apm)

@DeeDeeG
Copy link
Member

DeeDeeG commented Jan 18, 2023

Okay.

And I don't personally know what the goals are or should be for atom-community/atom, I don't have any big plans, but small little fixes like this I think are worth merging. So in the absence of any stated objection, I will do that in this case at least.

(May not finish tagging it and preparing it for inclusion in atom-community/atom right away, but soon hopefully. Feel free to ping me if I have forgotten part of it. And feel free to ping me here for updates on this particular fix getting to atom-community/atom.)

@DeeDeeG DeeDeeG merged commit 1d1c732 into atom-community:master Jan 18, 2023
@jonian jonian deleted the patch-1 branch January 18, 2023 19:21
@DeeDeeG
Copy link
Member

DeeDeeG commented Jan 18, 2023

Hmm, I noticed that at https://github.com/atom-community/apm/watchers, myself and some other person not directly affiliated with the atom-community org (to my knowledge) are the only two people getting notifications for this repository, overall.

I just sort of assumed other atom-community org members were getting notifications from here.

So I hope this doesn't come as a surprise to anybody as it makes its way over to atom-community/atom repo. It's a pretty trivial fix, though. And the concerns I could think of, regarding distributing as a GitHub tarball URL rather than an npm package, have been properly addressed, IMO. So I honestly can't think of why not. Anyhow, I do so only with the best of intentions, and I mean no offense -- I wasn't aware others had little chance to comment on this, due to not (apparently) subscribing to the repo notifications.

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.

Linux atom build failing (Arch Linux)
3 participants