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

feat: add 'properties' support #149

Merged
merged 5 commits into from Jun 15, 2018

Conversation

Projects
None yet
3 participants
@ldez
Contributor

ldez commented Apr 17, 2016

add support for 'properties' source.

I use java-properties but it's possible to use git-config for a better support.
What do you think ?

GitHub use git-config as support for the properties marker:

[user]
    name = John Doe
    email = example@examplecom

@ldez ldez changed the title from feat: add java-properties support to feat: add 'properties' support Apr 17, 2016

@ldez

This comment has been minimized.

Contributor

ldez commented Apr 19, 2018

Any news?

@daviwil daviwil referenced this pull request Jun 11, 2018

Closed

Iteration Plan: June 11 - June 22, 2018 #17501

8 of 13 tasks complete
@daviwil

This comment has been minimized.

Member

daviwil commented Jun 13, 2018

Hi @ldez! Thanks a lot for the PR. If you think using git-config will lead to a better result, please give it a try. Could you also add a test to ensure that the language gets parsed correctly?

ldez added some commits Jun 15, 2018

@daviwil

This comment has been minimized.

Member

daviwil commented Jun 15, 2018

Thanks a lot for making those changes! The tests are green, think this is ready to go?

@ldez

This comment has been minimized.

Contributor

ldez commented Jun 15, 2018

Do you want me to add more tests?

@daviwil

This comment has been minimized.

Member

daviwil commented Jun 15, 2018

Only if there's something else you feel is worth testing. Looks like you've added similar tests as the ones for other embedded languages, so I think it should be sufficient.

@ldez

This comment has been minimized.

Contributor

ldez commented Jun 15, 2018

Ok seems good to me.

FYI:

  • java-properties
    java-properties

  • git-config
    git-config

@daviwil

This comment has been minimized.

Member

daviwil commented Jun 15, 2018

Thanks a lot for the comparison! Definitely better to go with git-config, it seems. I'll merge this now and ship an update to be included with 1.29.0-beta0 🎉

@daviwil daviwil merged commit 4a79382 into atom:master Jun 15, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ldez ldez deleted the ldez:properties-support branch Jun 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment