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

Improve README files #52

Merged
merged 13 commits into from
Nov 16, 2018
Merged

Improve README files #52

merged 13 commits into from
Nov 16, 2018

Conversation

itaisteinherz
Copy link
Contributor

Just some meta tweaks to improve readability. I've also opened apertium/apertium-mlt-heb#1 to apply some of these changes onto apertium-mlt-heb.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 61

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 72.656%

Totals Coverage Status
Change from base Build 60: 0.0%
Covered Lines: 186
Relevant Lines: 256

💛 - Coveralls

@coveralls
Copy link

coveralls commented Oct 27, 2018

Pull Request Test Coverage Report for Build 74

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 72.656%

Totals Coverage Status
Change from base Build 60: 0.0%
Covered Lines: 186
Relevant Lines: 256

💛 - Coveralls

Copy link
Member

@sushain97 sushain97 left a comment

Choose a reason for hiding this comment

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

Some initial comments. Looks like there are some good improvements. You'll have to make to fix the build. Also, I think there is at least one other README you missed.

bilingual-module/README Outdated Show resolved Hide resolved
bilingual-module/README Outdated Show resolved Hide resolved
bilingual-module/README Show resolved Hide resolved
bilingual-module/README Show resolved Hide resolved
bilingual-module/README Outdated Show resolved Hide resolved
@itaisteinherz
Copy link
Contributor Author

@sushain97 For some reason the build continues to fail, even though I ran make (see ce07148).

@sushain97
Copy link
Member

I can definitely run make for you. But before that,

I'm not sure that using the fenced code blocks is the right approach. See below.

$ ./configure
$ make
# make install

fenced bash

$ ./configure
$ make
# make install

fenced console

$ ./configure
$ make
# make install
$ echo "TODO test sentence 1" | apertium {{languageCode1}}-{{languageCode2}}
TODO test translated sentence 1

fenced bash

$ echo "TODO test sentence 1" | apertium {{languageCode1}}-{{languageCode2}}
TODO test translated sentence 1

fenced console

$ echo "TODO test sentence 1" | apertium {{languageCode1}}-{{languageCode2}}
TODO test translated sentence 1

@itaisteinherz
Copy link
Contributor Author

@sushain97 I think I'll just remove the language hint whenever we have a command's output in the code block (since plain text that isn't a command always gets highlighted in a weird way).

@sushain97
Copy link
Member

@sushain97 I think I'll just remove the language hint whenever we have a command's output in the code block (since plain text that isn't a command always gets highlighted in a weird way).

Well, I believe using console instead of bash solves that problem. However, with either, the # that denotes a root command is interpreted as a comment...

@itaisteinherz
Copy link
Contributor Author

@sushain97 When I switched from bash to console in code blocks with outputs inside and looked at the Markdown preview, the text looked like so:

screen shot 2018-10-29 at 6 35 05

If that looks good to you, let me know and I'll add the console language hint to the appropriate code blocks.

@sushain97
Copy link
Member

sushain97 commented Oct 29, 2018

@sushain97 When I switched from bash to console in code blocks with outputs inside and looked at the Markdown preview, the text looked like so:

That is exactly what I tried above...

It looks fine for these.

@sushain97
Copy link
Member

Also, unfortunately your changes renamed all the files. That needs to be undone :\

You should be editing README, not README.md. The latter is just a symlink.

@itaisteinherz
Copy link
Contributor Author

You should be editing README, not README.md. The latter is just a symlink.

Are you sure? I couldn't find any symlinks in any of the folders.
I did, however, find such symlink in apertium-mlt-heb for example. I think we should add symlinks to apertium-init as well.

@sushain97
Copy link
Member

sushain97 commented Oct 29, 2018

Yes, I'm sure. I wrote the code that creates them myself: 0e3f199

@itaisteinherz
Copy link
Contributor Author

itaisteinherz commented Oct 29, 2018

Yes, I'm sure. I wrote the code that creates them myself: 0e3f199

👍🏻 I didn't think to look at apertium-init.py.
Is there anything else you'd like to to revise?

@itaisteinherz
Copy link
Contributor Author

@sushain97 Ping 🙂

bilingual-module/README Outdated Show resolved Hide resolved
Copy link
Member

@sushain97 sushain97 left a comment

Choose a reason for hiding this comment

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

I think the IRC links are broken? See https://github.com/sushain97/test-eng#help-and-support

@itaisteinherz
Copy link
Contributor Author

The link works for me.
We could use the webchat link instead: http://webchat.freenode.net/?channels=apertium

@sushain97
Copy link
Member

That is, it doesn't render as a link:

image

bilingual-module/README Outdated Show resolved Hide resolved
@itaisteinherz
Copy link
Contributor Author

That is, it doesn't render as a link:

GitHub doesn't render links with the IRC protocol:

[irc://irc.freenode.net/#apertium](irc://irc.freenode.net/#apertium)

is rendered as: irc://irc.freenode.net/#apertium

Copy link
Member

@sushain97 sushain97 left a comment

Choose a reason for hiding this comment

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

minor requests

hfst-language-module/README Outdated Show resolved Hide resolved
lttoolbox-language-module/README Outdated Show resolved Hide resolved
@itaisteinherz
Copy link
Contributor Author

itaisteinherz commented Nov 13, 2018

Because of 1d2a2d3, Travis fails for some reason. Maybe we need to run make again?

@sushain97
Copy link
Member

Yeah, almost all changes to this repo require a make.

@itaisteinherz
Copy link
Contributor Author

itaisteinherz commented Nov 16, 2018

@sushain97 It would be great if you could run make once more and let me know if there's anything else you'd like me to revise.

@sushain97 sushain97 merged commit 3d9db1f into apertium:master Nov 16, 2018
@sushain97
Copy link
Member

Thanks for your work on this, @itaisteinherz !

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.

None yet

3 participants