Skip to content
This repository has been archived by the owner on Jul 23, 2019. It is now read-only.

Contributing #15

Closed
max-sixty opened this issue Mar 6, 2018 · 13 comments
Closed

Contributing #15

max-sixty opened this issue Mar 6, 2018 · 13 comments

Comments

@max-sixty
Copy link
Contributor

Exciting project - thanks for sharing at such an early stage.

Are there areas you'd like PRs?
Are there good first issues?
Is there CI (and if not would you take a PR for one)?

@nathansobo
Copy link

Hey, thanks a lot for being interested! I'm going to take some time today to write up some initial "help wanted" issues on problems that feel contained and well-defined. I should probably also add a contributing section to the README. I'll leave this open until we've done a better job articulating ways people could help.

@jparr721
Copy link

jparr721 commented Mar 6, 2018

Would you be able to supply compilation instructions on the readme? I'm having some trouble getting the entire project to build and run as a single entity to test my changes.

@nathansobo
Copy link

Okay, I tried to improve the build instructions. Please open a specific issue if you run into snags building on macOS. On other platforms, I'm going to need people to help us figure things out and open pull requests.

@apcragg
Copy link

apcragg commented Mar 6, 2018

I was able to 'npm install' xray_electron on Ubuntu 16.04 LTS after installing libclang-dev
When I 'npm start' after the build I get a entirely white Electron window

@nathansobo
Copy link

Do you see anything in your dev tools console? Once you get it working would you care to contribute Linux specific instructions?

@mary-wien
Copy link

thanks, @maxim-lian for starting this. I would love to contribute as well

@apcragg
Copy link

apcragg commented Mar 6, 2018

I found an issue in scripts/napi.js

It seems that the {,} expansion works differently in linux than macOS (unix). I changed the script to work on both unix and linux based systems by removing the {,} expansion.On linux the file extension to be copied is '.d' not '.dylib' so maybe it should be changed to 'cp target/foo/module.d*' to capture both '.dylib' and '*.d'

@nathansobo
Copy link

@apcragg Sounds great! Could you open a PR with the change?

@apcragg
Copy link

apcragg commented Mar 7, 2018

@nathansobo Just opened one. I don't have a computer running MacOS so I couldn't confirm that this doesn't break the Mac build process. I'd appreciate if someone could look at that for me.

@daviwil
Copy link

daviwil commented Mar 7, 2018

I'll check it out on macOS real quick.

@daviwil
Copy link

daviwil commented Mar 7, 2018

Getting an error on macOS, will give more details at PR #25.

@nathansobo
Copy link

I've created a contributing guide and posted some help wanted issues. As we get more surface area and stabilize the architecture, there will be more opportunities. But I think some of the problems I posted could be interesting to people. Thanks!

@max-sixty
Copy link
Contributor Author

@nathansobo Thanks for the 'help wanted' issues, there are a couple v interesting ones (e.g. frag-ids and arabic text shaping).

That said, I imagine a few of us are keen Atom users and fascinated by Rust, but just starting out in Rust. If you have any issues that might be less 'interesting' but easier to implement, feel free to post those too

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants