[SVN] r7502 (Gettext)

Allan Odgaard throw-away-1 at macromates.com
Mon Jun 11 14:22:50 UTC 2007


# Abstract

These are review comments for the Gettext bundle submitted by Lawrence Akka.

Review comments should generally be anal, since half the idea with a public review process is to have other bundle developers also take note of the comments (and apply it to their bundles).


# Menu Structure

## Consistent Use of Title Case

The `msgid` and `msgstr` could be written out as `Message ID` and `Message String`. Considering how few items are in the menu, and how the other items are title cased, I would suggest such a change.


## Appropriate Use of Submenus

With only four items, there is no need for submenus.


## Good Use of Separators

The menu has two very different types of items (one set is move actions, the other set is “insert boilerplate” actions) yet no separator -- the two sets of actions should be separated.


## Good Logical Ordering of Items

Generally put actions (such as the move items) before items which insert boilerplate (i.e. snippets).

The main reason for this rule is that we generally have far fewer actions than snippets, and thus we want to start with these (i.e. place them at the top) so that they are not overlooked.


# Activation

## Proper Use of Scope Selectors for All Items

All items has the proper scope selector, good! :)


## Proper Adherence to Key Equivalent Conventions

The “Previous Untranslated” does eclipse Navigation → Go to Header/Source. I don’t think it is entirely unlikely that you may have `my_shell_command.cc` and `my_shell_command.po`, and thus want to use ⌥⌘↑ to switch between the two.

So I would recommend changing the keys to ⌃⇧↓ / ⌃⇧↑ (⌃⇧ are the designated _bundle item modifiers_).


# Language Grammar

## Using Only Valid Scopes

The bundle passes `Support/bin/validate_bundle.rb` -- so good job!


## Using Grammar Name As Suffix for All Declared Scopes

It prefixes all scopes with `po` and the grammar itself is `source.po` so this is good. Though I must add that `po` is a very short name -- what is it short for?


## Using Punctuation Scopes

The grammar does not use punctuation scopes, but then, I don’t really balme it ;)


# Bundle

## Understandable Naming of Items

The template should definitely be renamed from `PO file`. First, it is not title cased, second, it makes very little sense (it helps that it is in a Gettext submenu, but still).


## Contact Info in `info.plist`

Is there, good!


## Good Description in `info.plist`

I think we can elaborate on _is used for localising (sic) software_. For example what type of software? Is this system POSIX or similar?


## All UUID’s Are Unique

Passes that one as well.

Changed:
A   trunk/Review/Bundles/Gettext.tmbundle/Comments.mdown



More information about the textmate-dev mailing list