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

Tamil pages proof of concept #2512

Merged
merged 6 commits into from Nov 5, 2018

Conversation

Projects
None yet
4 participants
@arunisaac
Contributor

arunisaac commented Oct 28, 2018

This is a proof of concept for Tamil pages as discussed in #2339

  • The page (if new), does not already exist in the repo.

  • The page (if new), has been added to the correct platform folder:
    common/ if it's common to all platforms, linux/ if it's Linux-specific, and so on.

  • The page has 8 or fewer examples.

  • The PR is appropriately titled:
    <command name>: add page for new pages, or <command name>: <description of changes> for pages being edited.

  • The page follows the contributing guidelines.

arunisaac added some commits Oct 28, 2018

@sbrl

sbrl approved these changes Oct 28, 2018

Great work, @arunisaac!

The next step is client support. The official node client is over here - which looks like a good place to start. We should probably define a common 'specification' for how it's going to be implemented in clients - I think we had an issue for that somewhere.

Is there an environment variable that we could use to auto-detect?

@arunisaac

This comment has been minimized.

Contributor

arunisaac commented Oct 29, 2018

@agnivade

This comment has been minimized.

Member

agnivade commented Oct 30, 2018

This is the issue. Please comment on it so that we have everything at one place.

@waldyrious

This comment has been minimized.

Collaborator

waldyrious commented Oct 30, 2018

Shall this PR be blocked on having at least one client support the translated pages, or the other way around?

@agnivade

This comment has been minimized.

Member

agnivade commented Oct 30, 2018

I this it is the other way around. Clients cannot support translated pages, if this PR is blocked.

@sbrl sbrl added the architecture label Oct 31, 2018

@arunisaac

This comment has been minimized.

Contributor

arunisaac commented Nov 5, 2018

@sbrl sbrl merged commit 356b346 into tldr-pages:master Nov 5, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@sbrl

This comment has been minimized.

Member

sbrl commented Nov 5, 2018

Merged! I agree that client support is much easier to deal with if we merge this PR - we can always revert it if need be :P

Also, I've guessed a standard PR / commit message format:

cp,ls,mkdir,mv,rm,rmdir: add translations [Tamil] (#2512)
@waldyrious

This comment has been minimized.

Collaborator

waldyrious commented Nov 5, 2018

I've guessed a standard PR / commit message format

Actually @arunisaac specifically asked if the pattern he had used for individual commits (<command>: add Tamil page) was a good idea, and the agreement was that it was. I do agree your guess is a good counterproposal. Perhaps we could go with something like cp: add Tamil translation? What do you think @arunisaac @agnivade @sbrl?

@waldyrious

This comment has been minimized.

Collaborator

waldyrious commented Nov 5, 2018

On a separate note, I'm sad that @arunisaac's care with separating the commits was not reflected in the final merge into the master branch. The maintainers' guide suggests that:

If [...] the PR author took the time to craft individual, informative messages for each commit, then use the Rebase and merge method, to honor that work and preserve the history of the changes.

But I guess all these guides and documentation are becoming too verbose and more of a burden than helpful cheatsheets for day-to-day operation of the project. Or maybe it was my fault to propose them and not stick around for long enough to ensure they get assimilated into the maintainer culture (as other principles did). In any case, I hope @arunisaac does not get discouraged from making atomically separated commits in the future :)

@sbrl

This comment has been minimized.

Member

sbrl commented Nov 5, 2018

Oops! I think I missed both those points. On second thought, it would have been a better choice to rebase here rather than squash - I haven't rebased in ages on here! I'll try and consider this more in the future. I can revert & re-merge here if you like?

This conversation has gotten rather long here and in #2339, so I think I missed the bit on the commit message standardisation too.

@sbrl

This comment has been minimized.

Member

sbrl commented Nov 5, 2018

In terms of guides & documentation:

  1. Especially the client spec, I think a CLIENTS.md document is needed that details the spec there.
  2. Regarding the multi-language stuff, adding it to the maintainers guide (which was an interesting re-read, actually) sounds like a good idea. Maybe in a new sub-heading?
@agnivade

This comment has been minimized.

Member

agnivade commented Nov 6, 2018

I haven't rebased in ages on here!

Yes it's rare. But I rebase from time to time, when I see good commits.

Regarding, commit message, I think we should update the PR template too. Since that mentions how a commit message should be.

@arunisaac

This comment has been minimized.

Contributor

arunisaac commented Nov 6, 2018

@waldyrious

This comment has been minimized.

Collaborator

waldyrious commented Nov 6, 2018

I can revert & re-merge here if you like?

That would make the history even messier IMO. The only way to fix it would be to force-push to the master branch, but that's a pretty huge hammer that I don't think is justified in this case.

Perhaps, I slightly prefer cp: add Tamil page to cp: add Tamil translation since the former is slightly shorter.

That's a fair point, but IMO the 5 characters that we save are worth less than the extra clarity (not everyone knows the name of all languages in the world). So my preference still remains cp: add Tamil translation. Thoughts, @agnivade and @sbrl?

Regarding the multi-language stuff, adding it to the maintainers guide (which was an interesting re-read, actually) sounds like a good idea. Maybe in a new sub-heading?

I'm not sure the maintainers guide is the best place for that. Or do you mean the commit message format? Though even then, I think the general CONTRIBUTING.md guide would be a better place, don't you agree?

Regarding, commit message, I think we should update the PR template too. Since that mentions how a commit message should be.

Good point. Once we decide on the format, let's update that too.

@agnivade

This comment has been minimized.

Member

agnivade commented Nov 6, 2018

So my preference still remains cp: add Tamil translation. Thoughts, @agnivade and @sbrl?

Sure.

@sbrl

This comment has been minimized.

Member

sbrl commented Nov 6, 2018

Yep, cp: add Tamil translation looks good to me. On a related note, could we add a link to the maintainer's guide in CONTRIBUTING.md please?

@waldyrious

This comment has been minimized.

Collaborator

waldyrious commented Nov 10, 2018

I've made the discussed changes to CONTRIBUTING.md and the PR template at #2575.

I've added a link to the maintainer's guide in more places at #2576.

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