-
Notifications
You must be signed in to change notification settings - Fork 1
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
Release 0.0.1 #2
Conversation
@bradbumbalough I removed myself for now - until the time is right. It's an OCD thing. Feel free to assign/ping me when this bad brother is ready for review FOR REAL 🤝 |
3ffbfb4
to
eb11a95
Compare
@brianium this should be good to go now. I had to make a tweak when when wiring it into our mobile CI. |
workflows: | ||
version: 2.1 | ||
lint_test: | ||
jobs: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious why you opted for separate jobs for lint + test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really a reason... I could combine this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I care that much. It has been a minute since I have looked at the circle ui. I just remember workflows added a bit of complexity - i.e restarting jobs instead of the entire build. Maybe they have fixed this 🤷♂ - a job per task seems overkill to me but maybe I am just 👴
.circleci/config.yml
Outdated
lint_test: | ||
jobs: | ||
- lint | ||
- test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this some lawless land of no newlines?
.eslintrc.json
Outdated
"env": { | ||
"es6": true | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
....it must be
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tysonnero had a pretty cool integration of eslint and prettier in the desktop app. Might be worth looking at integrating prettier with eslint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. A couple of simple new line things and one request for prettier + eslint. The lint stuff could probably wait, but it would be nice to drive at a simple/consistent way to handle formatting + linting.
I think the real test for this stuff is going to see it integrating in the mobile repo.
.eslintrc.json
Outdated
"env": { | ||
"es6": true | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tysonnero had a pretty cool integration of eslint and prettier in the desktop app. Might be worth looking at integrating prettier with eslint.
@brianium made some changes... thanks for the feedback! |
d4c9e73
to
7f71838
Compare
Refactored the way the cli will work. Now, a single configuration file will contain the app schemes and version object.
Commands are:
expo-config generate [scheme]
expo-config version [type] [--commit] [--print]