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

added editorconfig, fixed style exceptions #582

Merged
merged 1 commit into from Dec 16, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 33 additions & 0 deletions .editorconfig
@@ -0,0 +1,33 @@
# EditorConfig is awesome: http://EditorConfig.org

# top-most EditorConfig file
root = true

# Unix-style newlines with a newline ending every file
[*]
end_of_line = lf
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't that in conflict with the default OS end of line choice? Git already manages text files depending on the OS, checking in or out text files with LF or CRLF based on the context. I'd prefer the approach of adding file patterns in a .gitattributes file to specify which one should be considered as text, and leave the default OS mechanism.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's in conflict with windows default, which is the point. I'm not sure I see the advantage of adding .gitattributes when this line of the .editorconfig should do the trick. The final result is lf in the repo.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not all editors have a plugin for .editorconfig, whereas the .gitattributes will be anyway be applied by git.
And it's more natural to allow the editor to edit the file in the native line ending.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally don't have a problem with adding this per se. It's fairly standard to add this to .editorconfig, as you can see from a sampling of major oss projects here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no problem, anyway nobody in her right mind is developing on Windows.

insert_final_newline = true

# Matches multiple files with brace expansion notation
# Set default charset
[*.{js,css,html}]
charset = utf-8

# 2 space indentation
[*.{js,json,yml,yaml,proto,sh,html,css}]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's also add our scripts without extension. I can only think about hack/build

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be easier to just rename hack/build to hack/build.sh

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one or the other is fine.

Copy link
Contributor

@subfuzion subfuzion Dec 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree. Library sh and bash scripts should have the .sh extension, but executable scripts should not. This is a subjective style issue, but I'm old school on this, I guess (even though I haven't always been 100% consistent myself). The only convenience for adding the extension has to do with using editors that require the extension for syntax highlighting. The Google Shell Style Guide agrees. Bottom line, if the file has a shebang line and the executable bit set, then don't add an extension. If the script is meant to be invoked by another script and is not executable standalone, then add the extension.

indent_style = space
indent_size = 2
trim_trailing_whitespace = true

# Tab indentation (no size specified)
[Makefile]
indent_style = tab

# Tailing whitespace
[{Dockerfile*,Makefile}]
trim_trailing_whitespace = true

# Matches the exact files either package.json or .travis.yml
[{package.json,.travis.yml}]
indent_style = space
indent_size = 2
2 changes: 1 addition & 1 deletion examples/ui/Dockerfile
Expand Up @@ -10,4 +10,4 @@ RUN yarn run build

EXPOSE 3000

CMD ["node", "server.js"]
CMD ["node", "server.js"]
2 changes: 1 addition & 1 deletion examples/ui/deploy.sh
Expand Up @@ -2,4 +2,4 @@
amp stack rm -f gw-ui || true
docker build -t examples/gw-ui .
amp registry push examples/gw-ui
amp stack up -f stack.yml gw-ui
amp stack up -f stack.yml gw-ui
2 changes: 1 addition & 1 deletion examples/ui/server.js
Expand Up @@ -8,4 +8,4 @@ app.use('/topics', express.static('public'))
app.use('/stackEdit', express.static('public'))
app.use('/dist', express.static('dist'))

app.listen(3000)
app.listen(3000)
2 changes: 1 addition & 1 deletion examples/websocket/stack.yml
Expand Up @@ -8,4 +8,4 @@ services:
image: 'registry.local.appcelerator.io/ws-bash-web'
public:
- name: web
internal_port: 80
internal_port: 80
1 change: 1 addition & 0 deletions project/CONTRIBUTORS.md
@@ -0,0 +1 @@

1 change: 1 addition & 0 deletions project/RELEASE-PROCESS.md
@@ -0,0 +1 @@

1 change: 1 addition & 0 deletions project/VENDORING.md
@@ -0,0 +1 @@