labofsahil/github-event-dataset-2012 huggingface dataset:
What is this bucket?  We have an images dir in the publish bucket, and now this.  We should be careful about a) pointing to s3 directly vs. our CDN; b) creating too many places that store the same basic content.
delete the name and projectID so that Butter doesn't get confused
Same, delete.
This terrifies me a bit, with the API key.  Can we somehow leave it out?
Should be `removeExtraHeadTags`.
same as above. test description doesn't tell me why it shouldn't delete. email is different?
Add "invalid_pdf_structure" entry to the l10n/en-US/viewer.properties file
combine with previous line
Wat???
Should not this be rather a separate PT story?

My initial intent was to have CPopupWindow::OnBrowserDocumentComplete(...) working for me even if m_pWebBrowser==0.
If I fix all m_pWebBrowser occurences in CPopupWindow class as you have suggested I VERY likely break something.
I think we should document it at least. But should we allow user use `insert` method for inserting last slide? He can use `append` method instead, but I'm not confident.
Sending JSON via the third argument of window.open seems overkill. Let's use the second parameter to specify the name and consider that every window.open calls from the homescreen tries to open a new window. Works for you?
Newline... ?
Not sure if i'm missing something.

It's keypos/keylen/malloc/read/free and datapost/datalen/malloc/read/free

Maybe the malloc/read/free could be put in a function that returns a string, but for line 82 it's "easy" to have the key as a char*.

Please advice if i simply missed something :)
Will we have a permissions model for the content search?
@parthaa, good question.  Actually, it doesn't look like we check the permissions for any of the items being added to the changeset (e.g. template, product, errata, package, repo or distribution).  There is permission checking performed on the list of items the user is able to select from to add to the changeset; however, if some crazy user decided to build their own REST calls, they could essentially add anything to the changeset today.

We'll need to solve this for all types (and not just content views). If possible, would prefer to handle that with a separate PR.
So #set_default_locale pertains only to the controller specs whereas this code will reset the locale for all specs (not just the controller specs). This code should fix the locale in the model specs where the failures are happening in Jenkins. 

The set_default_locale method is probably not needed anymore (although it might be nice to keep it around to see how to alter controller spec locales in the future). If it's not needed, I think we could remove it, create a good way to interact with the locale code in the controller, and also write some tests for the locale code. I should probably open a separate bugzilla item for that I suppose?
Isn't this a no-op given the previous `NSAssert` statement?
is there really any difference in having strict and warnings after or before the package declaration??
This existing functionality guards against join of multiple partitioned
tables where partition key is not part of the join expression. I factored
it out into a separate method because in case of union I call it multiple
times - once for the entire union and once for each individual select. My
reasoning was that even if individual selects satisfy the multiple
partitioned tables criteria the entire union may not.

On Fri, Sep 21, 2012 at 5:27 PM, Paul A Martel <notifications@github.com>wrote:

> In src/frontend/org/voltdb/planner/PlanAssembler.java:
>
> > @@ -182,14 +183,7 @@ private boolean tableListIncludesExportOnly(List<Table> tableList) {
> >          return false;
> >      }
> >
> > -    /**
> > -     * Clear any old state and get ready to plan a new plan. The next call to
> > -     * getNextPlan() will return the first candidate plan for these parameters.
> > -     *
> > -     */
> > -    void setupForNewPlans(AbstractParsedStmt parsedStmt)
> > -    {
> > -        m_insertPlanWasGenerated = false;
> > +    public void verifyTablePatition (AbstractParsedStmt parsedStmt) {
>
> spelling: paRtition
>
> I'm not following what this function is supposed to be doing.
> This is somewhat related to how ambitious you are about handling the
> various cases of combining child statements that are SP or MP over
> partitioned or replicated data.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/alexeevm/voltdb/pull/3/files#r1665946>.
>
>
Can you pep8-ify all your changes?  (http://www.python.org/dev/peps/pep-0008/)  You don't have to follow every recommendation if you think it makes the code less readable, but please remove white space before colons at the end of `def` lines, try to put spaces around binary operators, and so on.

(But note this method should be removed if we decide it's ok to break backwards compatibility).
I think this might be extractable from one of the nutation component functions in `earth_orientation`... but it might also be easier to just move this wholesale into a new `earth_orientation.mean_lon_of_perigee` function or something.
I didn't think we were at the time I removed it, but I was probably looking at something that consumed the final ring. It should be there.
Yeah, probably just my Java brain expecting a CommonClaimParams class or something horrible like that.
Because, apparently, some functions (I'm looking at you rt:stream_cmd) can't handle strings, only iolists. I'm going to go teach it about strings, the hard way.
It complains that this function is unused...
This I would rather change to `$model->recursive == 0` since only numeric values are meaningful for recursive.
why double-nesting bonus map inside bonus attribute?
you have used "hornetq.passwordcodec" in a few laces you should add a constant somewhere, same for above
Fixed in 332a14b.
This should be the default behavior, as the most likely other platforms (OS X, freebsd, etc.) will all behave the same as Linux here, rather than "Platform not supported".
you can just bind also notebook dirty to the worksheet-dirty event to avoid checking every time
file-based resources are hard to test and won't work in GAE.  Consider an alternate guice module or other pattern to get the InputStream that represents the keystore.  definitely, logic like this should be assignable in guice.
This makes no sense to me whatsoever.

~~~~c
#define GIT_MMAP_VALIDATE(out, len, prot, flags) { assert(out != NULL && len > 0 && (prot & GIT_PROT_WRITE || prot & GIT_PROT_READ) && (flags & GIT_MAP_FIXED) == 0); }
~~~~~
This docstring describes the old behaviour.

Also, if you are on the first page of a section, this will return the first page of the previous section. I was expecting this to return the last page of the previous section. @sgarrity do you have a preference?
attr="" or alt="" :)
generally this isn't ideal.  even though this is only 2 seconds.  that really adds up if it's used in very many places.  Better to use mocking.  See this as an example: https://github.com/mozilla/moztrap/blob/master/tests/model/test_mtmodel.py#L60
well our next identity solution is likely BlueVia w/ BrowserID. would we call that `v` or something arbitrary?
Does a javascript Number Object have a clone function??
Is there a reason that you are using an email field here instead of a Foreign key to the User?
why using both isset and array_key_exists ?
the return value should be removed once you switch to validate
Yes. `getMemberMetadatas` makes sense within the naming of `ClassMetadata` (we have `addMemberMetadata` for example) while `getPropertyMetadata` implements the interface.
Your test does not cover the issue. In fact, the `DataTransformer` is only tested with the `SimpleChoiceList` and not all the classes implementing the `ChoiceListInterface` interface. If you try to execute your new test without your fix, the test suite still passes.

To test the issue reported in #3727 isn't caracterized by your unit test. To reproduce the bug, you'd need to initialize the `$this->transformer` with an `EntityChoiceList`. However, this classe belong to the DoctrineBridge, so I'm unsure on where to put this test.
Please move this line under the update of $readOnly (103) and write it accordingly.
What´s that for?
should be ``0 ===``
typo here: ``pricess`` instead of ``process``
This is useless. Composer is already able to load the class thanks to the composer.json, even if the target-dir is used (it was not the case when the composer support was added to components at first though)
and it should be `@return string|null`
This is useful if you want to return `major`, `minor`, `patch` as integers. The reason for using ~~ along with `parseInt` is that `parseInt` can at times return `NaN`. ~~ makes sure you get 0 for any kind of falsey (`NaN`/`undefined` etc) output. Imagine having to get a major.minor.patch as `1.NaN.3`.
define toChannelBuffer instead, which would allow the implementation to avoid extra copies.
I need to find a better way to represent corrupt data. From what I understand, it isn't idiomatic to throw exceptions that are not future encoded within Finagle.
maybe `<% include foo.html %>` here too
I like that approach. My idea was to add a `timelineVisible` boolean to `Post`. Then the timeline only shows the visible ones, obviously. Users can decide whether or not they want their uploaded photo on the timeline. Then "deleting" a post from the timeline just means changing `timelineVisible` to false. All other photo functionality remains unchanged.
It looks like that those lines are duplicated with 295-297.
ghanoone copy-right ro reayat kardam :trollface:
Yeah, this is dead code.  Removed it in the "automation" (current development) branch.  Was thinking too far ahead (YAGNI'ing) when pairing.
This is incorrect. This is making a key called 'pType' that has a value of the function.
Yeah I was not sure about this one. If we agree to go with black and
white histograms and leave coloring outside the function, then
I guess it makes sense to go with 1 channel.
Why bother with PIL? Can we not just return a numpy array?
json is a stdlib import, and as such, it should be at the top. Then blank newline, then flask imports.
Shouldn't the  the target here should be `App.states` not `App.appStates`? If not then can you add a bit explaining how appStates translates to the `App.states` property below.
shouldn't that be help='the drop target ....')?
you're responsible for adding this then. If there is an issue, you will burn!
I couldn't get this to work .. any idea how to fix this? 

I thought it maybe had something to do with this: http://j-ferguson.com/scripting/escaping-bang-in-bash/ but tried the different approaches described here and couldn't get it working.
I don't know CoffeeScript much but this file looks like it can use some module pattern to get more reusable.
Not that much dangerous cause in the beginning of the script the opencv directory is removed and created from scratch, therefore we know what's there.

I tried to force extracting to a directory on the tar line but it extracts stuff to a subdirectory of the directory I specify, which is not what we want.
Should this be in here? @jdalphon
Clean up is done in tearDown(...) call: super.tearDown(managementClient, containerId); 
c.f. org.jboss.as.test.integration.security.common.AbstractSecurityDomainSetup.tearDown(ManagementClient, String)
Brian noticed that SecurityDomainAdd also does not handle MODULE_OPTIONS
The cause could well be null here, a lot of services throw StartException directly.
This should just look at Transition.getAfter(), e.g. there is the transition STOP_REQUESTED_to_UP that is missing (even though it is not very common), and it is possible that the state machine states and transitions could change in future.
You've changed this docstring by accident I guess.
odd indentation on these 2 lines?
nit: Break line if it is more than 80 chars. Also I guess that you should check the patch with the linter.
The point is that the "confirm" BB code for the messages is in the dialer's index.html file to cover all the app when the dialogs are shown. The use of the "parent "is due to the Contacts being an iFrame in the Communications app. I hope I made myself clear ;-999
nit: don't think we really need those comments
Beware that it would throw exception when callback is undefined(I know it won't happen in our app now).
We actually have a `mock_system_banner.js` if possible I'd like to make it evolve instead.
Why don't you save the alarm object that the view module got from `load`, update it, and save it to DB, instead of create a new one and collecting everything from UI?
`vibrate`
Oh you wascawwy wabbit! ;-)
As you know I’m not fond of using Date() methods to format durations, but I guess it’ll work with any timezone — which is your concern.

r=me, merging
Sorry yes you're right of course. Don't understand my own code ;)
`attentionScreenApps`
We need to add fork=True for all the Storm commands, right? Maybe it would be better for fork to default to True, since the number of commands is likely to grow faster than the number of daemons.
この空行の追加は不要です。
You need to copy the option before messing with it, otherwise other extensions that use the option will get your changes.
I wasn't sure about that. I like these names.
Looks good to me.  I am trying to make as few changes as necessary to initially integrate Faraday, but I think we should open issues for any places that could use further refactoring.
I don't think scala will be able to fine this Monoid implicitly without additional code.

Can you remind me again why you don't lift the SparseIndexedSeq trait up by itself, then have private [algebird] instances that you have here, and use the SparseIndexedSeq companion object to create Empty and get started from there?

With that approach, an implicit def monoid[T:Monoid]: Monoid[SparseIndexedSeq[T]] would be found automatically (and code like Monoid.plus(v1, v2) would just work).
So since this is a var and not a constant anymore, maybe we should not use all caps?
oops. ill fix that after you look it over. lol
Move this up next to the get request handling.
I think having a # would be confusing, in my opinion it would be better just to have the user enter the id.
Should this be:
   } elseif ( $this->me->isAdmin() ) {

I mean, the code has no effect because of it never runs when it can be true (all admins are moderators).
There isn't really a fail, only a shortcut without starting a queue.

Cool, hope this will be useful!
Agreed. That's the way the code was when I inherited it but I've changed it now.
This should be a lager or error_logger call.
I think it is a very common need for users just to want to create a function to do the interpolation. Why it is not possible to just create an adater from callables to IRenderable such as I can say:

    Shell(command=lambda p:"echo next build is"+(p.getProperty("buildnumber")+1))
This close does nothing.  with scopes its open/close for you.
Doesn't this make the assumption that all use defined constants are going to be after LOG_ERROR?  If they aren't they will be omitted.
@Seldaek Fixed, Can you test it, please? We should to find better solution. Each type of class loader require different order of packages:

    Available packages:
        a (requires: c), b, d, c (requires: d)

    Main package requires:
        [a, b]

    Packages in repository:
        [a, b, d, c] or anything else

    PSR0 loader requires ordered packages:
        [a, b, c, d]

    Classmap and files loader requires reverse ordered packages (same as in installed.json?):
        [b, d, c, a] or [d, c, a, b]?

This PR solves reported issues but doesn't provide complex solution.
could make this a class method on KeyRing, which just take a hash and returns an object
This is unnecessary code. These params are optional, which means that you don't need to pass them unless really want to set them.
Can we change it to `/projects/:id/repository/commits/:sha/blob`?
Cool. I rebased and there is a few things that stopped working. And that's one of them!
`params[:branch].presence`
I don't think that `export ...;` is needed, why just not adding `RAILS_ENV=blue` before `bundle exec` ?
Yes, thats right I wrote some documentation... It was bugging me :smile_cat:
Seems start is access w/o any proper bounds checking? I think we need to have a check for minimum packet size required for Teredo.
Wonder if we should use a separate event for this.
plen check is missing. We can't assume the buffer to be big enough for even this check.
Perhaps use `${%GlobalSecurityConfiguration.DisplayName}`?
Should this be #__contact_details? Shouldn't it be a banners table?
Was this renamed to avoid a conflict with an existing method? I regret monkey patching because of this problem. Since this method is only used once, Perhaps this method should be removed all together and the call in `abstract_val.rb` should be replaced with:

```ruby
@children.map(&:size).inject(0, &:+)
```

This will return `0` on an empty list where `sum` would've thrown an error, but I don't think that matters here.
Missing capital `i` in `if the foo() promise`. And it looks like there is a duplicate `propagate`.
the error propogate will propagate -> the error will propagate
Could you please move imports to the top of the file ? I think you're trying to optimize here, but I think that JSM will be loaded anyway and require chrome is cheap.
Please make sure that exporting **ChromeWorker** like this actually works, since as pre-existing comment suggests, function wrapper was intentional to workaround some strange platform behavior.
This is a generic base class, so "to create" is not appropriate in this message.
apparently not - http://expressjs.com/guide.html#res.send%28%29 they're saing you need to use res.json() only if sending a non JSON variable type and want to force JSON mimetype
Why not copy the original exception? For your test case below, I get 

    <DBus::InvalidParameters> exception expected but was
    Class: <ArgumentError>
    Message: <"wrong number of arguments (3 for 1)">

and

    <DBus::InvalidParameters> exception expected but was
    Class: <ArgumentError>
    Message: <"wrong number of arguments (0 for 1)">
Can users directly modify these?  It seems like you don't expect that, but I am not sure.  If not, please disable direct modification of those fields.
Dropped `ScopeDecorator` as suggested.
`head` is actually not a head, but a `ref` (reference to a commit), if I understand you correctly.
`(strlen("") === 0) === empty("")`, no need to change this
Yep, as @ralphschindler said
Space missing after ```if```
approximés, sans e
all 5 previous lines should be:
```ruby
labelIds.empty? ? []  : labelIds.split(",").map { |id| Label.find(id) }
```
is it cloned as the ssh version, though, or the http version? (i.e. we can write to the ssh version, but not the http version) if the subrepo uses the ssh version then we can just use that. Or we could modify the .ssh/config in the subrepo or something...
That's good, but you forgot to remove the other location of `myModules`
Sorry, my mistake, this is exactly how they're meant to be used...
i wasnt sure how to best name the class and namespace in order to follow the spirit of the rest of the code ..
For consistency I'd move the declaration of `comp` within the `for` block.
Application.__init__(self, *args, **kwargs)
@SessionScoped should have backticks
I'd feel better if `transformFunctions` was cloned instead of referenced here (`transformFunctions.slice(0)`).  The code in this Pull Request doesn't modify `_transformOrder`, but tricky bugs may arise if `_transformOrder` is ever modified internally.
Please rename attribute to "dialog-modal", because it is related to dialog. Imageupload can be also used inline, without dialog
Add a space between the first '>' and '.row' for readability? (ll. 22 to 27)
This invalidates this test. The `nonvouched_only` should work even if there is no actual query: that's the point, to list all unvouched users.
This assumes that `PERMISSION_LEVELS`  is sorted by priority and that higher levels will always have the permissions of lower levels. 
This will break if `PERMISSION_LEVELS` is reordered or if a new permission doesn't follow this scheme.
For the code to reach this point `move_files` or `rename_files` (you want to leave the original files untouched when they're renamed, don't you?) have to be True anyway.
It's not a magic number, it's a variable configurable from sakai.properties. Maybe the name should be lowercased.
main.py was testing to make sure that it worked.
Depending upon whether you told the vm to 'start' or 'stop', you need to wait until it reports completion of the action you requested.

I suppose you could write this as 

vm.wait_for {(action == "start") ? vm.ready : !vm.ready }

But that did not work as reliably when I was testing this, can't remember why.
@ohadlevy, correct. it defaults to 389.  I will remove :required
of course. I had just forgotten to save the tests, thanks for catching that.
I'm not completely sure what you are synching on here, but maybe the Stat thing should be inside the synchronized as well.
As per the [Gem guidelines](https://github.com/alphagov/wiki/wiki/Gem-guidelines), bumping the version should be done on master.
fccs
Uh, yeah, I'll do this camelcase next time, can change now if you want.
Doesn't catch the case where the finder actually returns just the whole collection. This has actually happened to me with find_by [it returned the first object]. See below.
Maybe a better hint like in the debug error message just above (adding `$phpScript` and/or the template name) ?
`isset( $selected[0] )` checks quickly if the first element of an array exist. If so you also know that you have the array.
You can simply do one check `isset( $selected[0][1][2][3][4][5] )` instead of many `is_array`.
а зачем пробелы вокруг стрелки?
а graph_size существенно короче graph->size()?
как-то тут много лишних вызовов сравнения
надо написать, чтобы понятно было, что происходит
Not sure if it makes a real difference, but inside gd code we should probably continue to use gdFree
Ok, saw the comments regarding this in rc_setpoint.h
This should be factored out to a common function if it is needed in more than one place...
Also since you need all the euler angles multiple times it makes sense to get them only once (each call to the get function has at least the overhead of checking if it's calculated)
```c
struct Int32Eulers att_eul_i* = stateGetNedToBodyEulers_i();
if (abs(att_eul_i->theta < INT32_ANGLE_PI_2) {
}
```
:-)
Only you guys currently have RC_MODE commands with the quadshot. You don't have that for any of the normal quads, so this would break all of those.
Making command laws taking mode as input would be nice, but it might already be sufficient to move this function somewhere else and only call it when you get a new RC frame, are in some sort of manual mode (e.g. not full nav) and before you set the other commands.
Or maybe use ```SetAutoCommandsFromRC``` and put commands that should be set from the RC in any mode in the ```<auto_rc_commands>``` section.
please all caps for defines, also in the module it is capitalized
Is this fixing a bug?
Sorry, I don't agree. Compounds with proper nouns [should also be written as a single word][1], with an optional hyphen for readability. So it's _Refinery-gebruiker_, just like _Facebook-profiel_, _Albert Heijn-filiaal_, _Hemaworst_.

[1]: http://www.onzetaal.nl/taaladvies/advies/rabobankrekening-rabobank-rekening-rabobank-rekening "Rabobankrekening / Rabobank-rekening / Rabobank rekening, language advisory, Society for our Language / Genootschap Onze Taal"
Missed the indentation for this one. Also there's a multi-line param above.
This shouldn't be in this file.
This can probably be a separate sprint item?
Do we need to assign this and above functions to a variable?
I think we can probably improve the line breaks in this entire test to make it slightly more readable. I'd make every test goal a block with no line breaks, and do a line break after the block is done

e.g.
```
// Comment as manager on (private) piece of content (--> success)
test ...
test ...
test ...

// Comment as member on (private) piece of content (--> success)
test ...
test ...
test ...
```
I thought of that too but decided against it. If I used super, I would receive a hash with the content of all row elements. Then I would have to replace a bunch of values of the hash with the values of the input fields.

Let's look at a table with three columns: movie title, artist and duration. The title is just text and the other two columns would have input fields. The movie title is 'Forrest Gump', the value of the artist field is 'Tom Hanks' and the value of the duration field is '136 min'.
```
#this is what super would return
{'Title' => 'Forrest Gump', 'Artist' => ', 'Duration' => ''}
#and I would like to have
{'Title' => 'Forrest Gump', 'Artist' => 'Tom Hanks', 'Duration' => '136 min'}
```
The super method gives me a lot of data that is useless for the table form. I would have to reprocess more data than I can actually use.
考虑一下循环输出，代码总有办法 dry，而且，1,4 这种也需要把所有项变成 1,3 如果删除一项的话；另外，从语义上看，3-1 这种更好理解。大家都知道是3个中的第1个
Will do that in my local.
For both banned blocks and banned items, the if statement is only ran if the player doesn't have the proper permission. There may be loopholes around this, but I have yet to find any.
Lijkt er op van niet. Weghalen dus
Да я ни в коем случае не придираюсь! Я для себя пытаюсь понять в первую очередь.
3 года Python очень отучают от статической типизации.
Можно использовать characterAtIndex
Can you use single quotes here & below?
Yeah I think that's fine. If someone complains I can fix it.
```
projs : list of Projection | None
    If not None, it will replaced raw.info['projs'].
```
Same for result.weatherdata.weather, result.weatherdata.weather[0]
Or use same approach as SingleClassHibernateDAO (two constructors).
I think it may not be enough, e.g. JavaScript has `application/javascript` mime type and JSON has `application/json`.
Same note as above re `row/colCount`
Fixed in e9a00ef
Although it isn't part of the Coding Standard, it is better to put an empty line between the properties and the methods.
But, i wanted to seperate components. Is it ok for tweetDlgComponentContainer??
Likewise with `BCrypt`, this should implement the same mixin interface as `SHA1`.
Then define it where it's needed! Messages.scala is probably the correct place. As a bonus, we can get rid of the silly `stirling.itch.common` package.
I suggest leaving it as is. Removing only one set of parens like the last example looks to inconsistent to me.
when I think about it even if multiple threads call start/stop it will be fine so in a sense it is TS even though wasn't designed to be.
This methods are only used by CamelExtension in controlled environment so we are safe here. I wanted to avoid adding "started" guard as the assumption was this class is going to be used by camel extension only.
Okay, I see now. Then this looks good to me.
@eagleoneraptor no problem. The empty() function IS a tricky one.

Other than this, I think we are fine. Please add some words about $default parameter in the API so that people know if it is null, it means no default value is provided.
Ага. Строка, которая парсится в рантайме, меняет стиль передачи аргументов,
который делается компилятором в компил-тайме. Херню тебе сказали.
cleanup this mess
Should be prefixed by  "get" 
I think this method name is not appropriate as it returns an array of collections ... 

what about getAllowedBusinessFieldCollections ?
@romainneutron Fix typo here 'toen' -> 'token'
constant please
Right, fixed in the updated commit
Good lord. Horrible variable name.
Perhaps move the `for rgba` part of the list comprehension into this line?
Same with the other patterns.
This is going off of what I learned from doing media automation from Roc - you can't reliably rely on sleep calls over a time period to be entirely a 1:1 mapping to the time passed in the UI. You can reliably do things such as capturing and analyzing media events, detecting a UI change to a new time, etc.
What's the `time.sleep` here for? This probably deserves a comment.
Primitive::rule($name)->
    setExpression($rule)->
    setForm($this)
I've NO idea what you're trying to say me by this jdoc
I thought I had landed this, but luckily I haven't. Sorry about the false alarm. I'll make those changes momentarily.
@bp323 please fix the spaces & double quotes
Its a temporary API, the whole system around SiteTree->getChildrenAsUL() needs refactoring, so I didn't want to get into premature optimization. Also didn't have time to performance test how much SS templating would add to tree rendering (I expect its significant, given the number of rendered nodes). So left it with string concat.
Maybe add something like "You can retrieve a session key by calling dev/tests/startsession"?
so, I don't think we need rx in the tests of daybed.
I changed this because it didn't seem to remove all but the newest build.

Now it sees that there is more then 1 build and will remove all the rest.
In scala, all variables/functions without specifying the visibility are public. Since most elements are wanted to be public, this reduces the writing effort. But you then must write "private" to variables that should be.
Are you sure you want to override the parameter ``$event``? This often makes code less readable.
On 05/22/2012 02:02 PM, Nick Anderson wrote:
> Yeah I considered having two different usebundle promises classed
> inversly on the presence of the _usebundle_parameter_if_critical etc ..
> if it was defined but that just seem to complicate things. It can always
> be added. I thought that requiring a parameter for the bundle was a
> little heavy handed.

Actually yeah this just doens't seem like it would work passing 
parameters. I don't think there needs to be any special support in the 
sketch to use a dynamically generated bundlename.


  !! A method attempted to use a bundle 
"bundleifok2("main.check_dummy_ok2_conf")" that was apparently not defined!
I: Report relates to a promise with handle ""

So that qustion will just have to be a "wont fix" since usebundle cannot 
be a string that expands into a parameterized bundle.
That's how the Routes library does it, which in turn comes from Rails. I think it's because `/packages` makes sense for listing all packages, and you want the URLs to match.
Because of the way the code is structured, I can't think of a good way to implement it. Mongo::MongoClient.from_uri does make the connection, however how does one then set the database from the uri and authenticate from the uri?
Should this be setting `aria-hidden` to true?
Perfect; please make the change.

On Oct 29, 2012, at 1:01 PM, Adam <notifications@github.com> wrote:

> In content/v2/payloads.md:
> 
> > @@ -51,7 +51,9 @@ ctime
> >  
> >  custom_data
> >  : _String_ of up to 1K of client-specific data that may have been sent when the job was submitted.
> > -mt This value will be "1" if the text in body_tgt is a machine translation. A machine translation is provided as a convenience while a human translation is pending. You can choose to use or discard the provided translation.
> > +
> > +mt
> > +: _Boolean_ of whether the content in body_tgt is a machine translation. A machine translation is always provided as a convenience while a human translation is pending.
> How is
> 
> : 1 (true) / 0 (false) of whether ...
> 
> —
> Reply to this email directly or view it on GitHub.
> 
>
Eh, let's not break any more than we have to when it's something that small.
It was there before (slightly changed), so I just left it there...

Hmm... I have too much confidence in other people code :)
Does not tags have automatic access to template context? You don't need to pass request here?
Good point.  The else case is greater than right?  I think if DB_VERSION == get_version() we don't need to log anything.
Yes, this would violate that guarantee. Do you mind reminding me why it is important that escodegen makes that guarantee?
It should be `-ifndef...`.
Stimmt ganz vergessen, das printf-like zeug...
done in next pr
This should have a class of page-heading - this is from bootstrap. It's not the site header but a page heading. Good place to use that class.
Hey, 
This was commented for a reason. The play area does not reder correctly. It will need to be fixed prior to being used.
I'm not sure what do you mean by that as I don't know how to pass the other parameters to the function in the preg_replace_callback below this. I didn't liked the approach but atm I don't know a better one.
¿Has ejecutado la batería de tests  de users_skills_test.rb después de los últimos cambios? El if bd debería terminar con un "end", no con un elsif y de esa manera que el primer mensaje de "Recibido permiso..." se ejecute en el else de "if self.frozen? # quitando permiso". Creo que te dije yo que pusieses elsif, perdona, no revisé el código lo suficientemente bien, no me mates o:) Generalmente trato de dar buenos consejos pero cuestiona todo lo que te aconseje y si en algún momento ves que digo que 1 + 1 = 3 dime: no, slnc, 1+1=2.
Oh, I had meant to tag the maven-release-plugin config below this line. Sorry for confusion.
what happens if this hook fails ? do we want to catch the error and just display it ?
But if we have an IOException while recording I don't think it is gonna be valid... but sure, I added this check
But from looking at the GLIB code it should generate a warning if g_error_free is passed NULL.  We don't want the warning as we pass NULL when everything is actually okay.  In anycase, everywhere else in the code we check to see if error is NULL before calling g_error_free (if it can be null) so we might as well be consistent.
THE TAG IS DEPRECATED? O NOES
![sad-cat](https://f.cloud.github.com/assets/514769/8303/e0af3cb6-446d-11e2-9bf6-83023575f7e9.jpeg)
Barney is checking e-mail while he's away, so let's wait for him to respond.  If he agrees, then I'll get rid of it on this branch (from galsim/, tests/, and examples/)

On Aug 13, 2012, at 10:07 AM, Mike Jarvis wrote:

> In galsim/base.py:
> 
> > +    -------
> > +    The DoubleGaussian is a GSObject, and inherits all of the GSObject methods (draw, drawShoot,
> > +    applyShear etc.) and operator bindings.
> > +    """
> > +    
> > +    # Defining the descriptors for storing object parameters
> > +    flux1 = descriptors.SimpleParam(
> > +        "flux1", group="optional", default=None,
> > +        doc="Flux for the first of the two Gaussian components of the DoubleGaussian.")
> > +
> > +    flux2 = descriptors.SimpleParam(
> > +        "flux2", group="optional", default=None,
> > +        doc="Flux for the second of the two Gaussian components of the DoubleGaussian.")
> > +
> > +    sigma1 = descriptors.SimpleParam(
> > +        "sigma1", group="optional", default=None,
> No objection here.
> 
> —
> Reply to this email directly or view it on GitHub.
> 
> 


-------------------------------

Rachel Mandelbaum
rmandelb@andrew.cmu.edu
http://www.andrew.cmu.edu/~rmandelb/
This is admittedly a shorter code block, but it does get repeated a lot both for the FWHM and sigma tests, so might be worth making into a function?
The `setFlux` method on the `GSObject` classes seems to still operate in-place (even though the `SBProfile` one does not).  I think the original line is the correct one; the new one sets `target_PSF` to `None`.
Agreed that versioning is good. Seems more common to have something like thumbnails_201201121200.js or something.
These classes _are_ in master.  I made the model class definitions in master and then "backported" to fall2012-master, precisely because we'd have two codebases sharing one database.

On Dec 8, 2012, at 8:48 AM, Joe Blaylock <notifications@github.com> wrote:

> In main/c2g/models.py:
> 
> > @@ -1844,6 +1836,16 @@ class ExamRecordScoreField(TimestampMixin, models.Model):
> >      associated_text = models.TextField(null=True, blank=True)
> >  
> >  
> > +class CurrentTermMap(TimestampMixin, models.Model):
> > +    course_prefix = models.CharField(max_length=64, unique=True, db_index=True)
> > +    course_suffix = models.CharField(max_length=64)
> > +    def __unicode__(self):
> > +        return (self.course_prefix + "--" + self.course_suffix)
> > +
> > +class StudentExamStart(TimestampMixin, models.Model):
> > +    student = models.ForeignKey(User)
> > +    exam = models.ForeignKey(Exam)
> > +
> But I just realized it may look like it came in from master because you merged my other PRs which also had this commit on them, so it may have derived from someplace else originally. The other likely candidate is perhaps this change made it into dca_work_new right before I pulled. That makes sense actually because he was starting to work on the exam things on this branch just as I was setting up to work on parent/child, and I wasn't particularly discriminant about which commits I pulled in from him.
> 
> There must be some one-liner to say 'follow this commit back and tell me where it started from, no really'.
> 
> —
> Reply to this email directly or view it on GitHub.
> 
>
Why are you doing string manipulation instead of using System.IO.Path methods?
same here, we need to get rid of all $_POST variables
This pull request will need rebased against master, I presume.

It also includes tomcat7 changes that have already been merged from a different pull request.

I won't try to merge this until you let me know that it's been rebase, and is thus ready to merge.
This is already applied in HEAD.
I am not against making this more sane for specific needs. 

This file has since been turned into a template (our bad for not merging reviewing your PR earlier) at `templates/logrotate-httpd.erb`, so the best way to play on these settings would  be to set them using the template. I would welcome a patch to pass such parameters to the template.
I guess the `()` are not needed now.

eg:
```js
// this looks better
shape.group = GROUP_BUGGY;
```
`float /*double*/ ` should always be replaced with `CGFloat`, you should be able to do a global search-and-replace.
Again, should that really be `.isa`?
Use cmath
If you move above code to webinos/test/policy this line should be changed to path.resolve(path to webinos/tes/policy, filename);
Let me add a `XXX` here?
It's not slower, just a readability improvement.
why did you change this to 1 ?
Do you mind commenting on my latest pull request?
yeah basically.
I don't feel inclined to merge this.. The "src.x" syntax is very readable and ensures that the reader understands instantly you're referring to an instance variable, rather than a local. I don't want to force anyone to use "src." anywhere, but I think if a coder decides to use it, they've had a reason, and we should respect it as to ensure readability of code.
Si aici ar putea fi apelat constructorul de pe L40.
Yeah, that sounds like a good idea.  I'll keep that in mind for another patch set that does this for F1/F3/F4.
I don't like it, but it's there to preserve the behavior of the existing singleton.  The way the current code works is all new configs extract values from the current config, so everywhere, unit tests etc. depend on this.
Class variables are pointless as you are not interested in inheritance or sharing variables between instances and classes.  There's no reason to not just use an instance variable on the class.

Example:

```
1.8.7 :001 > class A
1.8.7 :002?>   @@blah = "a"
1.8.7 :003?>   end
 => "a" 
1.8.7 :004 > class C < A
1.8.7 :005?>   @@blah = "c"
1.8.7 :006?>   end
 => "c" 
1.8.7 :007 > class A
1.8.7 :008?>   @@blah
1.8.7 :009?>   end
 => "c" 
```

versus:

```
miles@monkfish ~ $ irb
1.8.7 :001 > class A
1.8.7 :002?>   @blah = "a"
1.8.7 :003?>   end
 => "a" 
1.8.7 :004 > class C < A
1.8.7 :005?>   @blah = "c"
1.8.7 :006?>   end
 => "c" 
1.8.7 :007 > class A
1.8.7 :008?>   @blah
1.8.7 :009?>   end
 => "a" 
1.8.7 :010 > 
```

In my opinion, there's really no reason to ever use class variables in Ruby, they just cause surprising problems.
По-прежнему название директории с большой буквы.
Зачем закрывать в catch?
А если будет null?
s/Catergories/Categories
we have source control, so why checkin code that's commented out?
既存の和訳ドキュメントの目次でDataSourceはデータソースと表記しているので、カタカナのほうがよいのでは。
I can take on the translation, but most likely not until this weekend.
nope - should be a member (inherited from MulticlassLossFunction?) - I'll look into that tomorrow.
Not important since you don't use it in the outer loop, but you are redefining the i variable in the nested loop
`self.random_state` should be used instead.
Makes sense, will do!
Can you implement the tests for the multiclass and multilabel cases in two different functions? `au` is not  used in the multiclass case.
No need to sort; `np.unique` guarantees sorted output.
Not quite clear about this, what does this refer to? XP Framework was originally founded in November 2001, and released in February 2002. Or is this the date this maven project was created?
Yes, they will be converted. I didn't do it before merging this because Stuart was doing similar stuff and I didn't want to conflict. Plus it was late.
В этом случае текст в большинстве случаев содержит несколько подряд запятых. Нужно фильтровать пустые.
Плюс необходимо проверить, что будет в афише, после этих изменений. Мы ведь там отображаем этот адрес.
All I'm saying here is "maybe" - I'll have to have a quick look at what makes up classinfo.init.
should be `"TableViewRow"`
it was removed unintentionally
Changed to bb::cascades::Color::Transparent
s/html5/HTML5/
Le temps d'éxécution de la requête exprimé en secondes
Please keep in mind that lines have a 80 characters soft limit
So, in standard mode, if we have multiple listeners, some WorkUnit, some not, having the workunit listener with dynamic resolution won't help much, unless it exclusively handles files that the others don't. Nevertheless, work unit listeners can set the dynamic flag directly on the query factory without the need of any flag set in main.
Ignore the comment, it's left-over from older times... I'll remove them...
Well, it's number of threads in the threadpool, as far as the eclccserver code is concerned... where each thread in the pool may spawn a process.
It's mocking what `requests.get` returns.
Use `reverse()` here instead and here you don't need to use `&amp;`
This is a bit tricky but I guess it works in irb, so...
Почему не rails 3.2.8?
seems more like a debug logging level
not clear how this will interact with OR pipeline, I feel like if/else might be clearer
You should make this check after you've confirmed device.platform is Android - we don't know how other platforms structure their version number (once can imagine something wacky like 'v1b'). Other than that this looks good.
ah - I saw some small changes (changing `value` to `values`) that I thought might be a problem for 0.8.9. But if it works, it works.
I think resque namespaces all the redis keys it uses, but yeah, that's a good point.  I'll update it.
You can probably use a for to iterated through them instead of manually.
@raydavis  We should have consistency across APIs on whether missing fields are missing or false. So far we've been using true/false, but I can go either way and don't have strong feelings. True/false is nice because you can see at  a glance what's missing.
This should fail the task if this helper fails. All the logic for ending a task should be inside the task.
PHP does not have named arguments. So using an associative array is the best way to create issues later by making the user think they can add the new arguments in the order they want whereas the order is in fact important.
Thus, the YamlFileLoader does not call ``array_values`` (because it assumes people don't use associative arrays when they should not, which may need to change) so using an associative array would cause some issues when trying to modify arguments later (through a compiler pass or when using service inheritance)
Ideally this should be:

rails generate refinery:settings
@bodepd : Sould this dependency be set in the swift::ringsync define ?
Are you checking errors when running winrm_run_cmd?? Not checking errors and returning if something goes bad can generate exceptions, as sample (notice the exception after the print_error):

<pre>
msf  auxiliary(winrm_cmd) > rexploit
[*] Reloading module...

[-] Login failure! Recheck supplied credentials.
[*] Error: 192.168.1.135: TypeError can't convert String into Integer
[*] Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed
</pre>
Should one of these be ```o_words - s_words``` ?  Looks like you're just getting two copies of each thing in ```s_words``` that isn't in ```o_words```
```then``` is superfluous
`_a1` and `_a2` unused?
I suggest we move this logic into `EnsimeApi` - into something like `select_subproject(self, on_complete)`. One of my nightmares was disentangling client/server interaction, so it'd be great if we could keep as much logic away from the controller as possible.
Wouldn't it make more sense for `computeScreenSpacePosition` to return the center of the label rather than the lower left corner?  It seems easy enough to calculate and would be more useful.
It's a user defined color that is blended with water color when the specular map value is not full 1.0
Who cares if it is a point or a line?  We could make a similar argument for a sphere with a radius of zero.  Unless there is a compelling reason for the exceptions, let's do as you plan and be as flexible as possible for the user.
Are you using this to convert from 64-bit to 32-bit floating point?  I had the same idea a while back.

Perhaps it justifies its own function.
I see why we remove and add with the map here and in `setOutlineWidth`, but don't we think that it breaks abstraction?  Why does the polyline need to know that the collection has a map?  Why does it have to know to call `makeDirty` when some properties change, and manage the map when other changes?  Can we move the logic to `makeDirty` and still keep things fast and clean?
_updateHint is a method on the child class TextField, meaning that this will code will crash if you use a TextArea.
Is there a default value for this property? i.e.,
default: true (Tabs displayed at top of the screen.)

or

default: True on Android, false on iOS.
split android... into separate path.join args.
just do \<!DOCTYPE html\> to specify HTML 5 and save bytes. No reason not to.
I am not sure if we support password mask on TextArea. It doesn't appear to work during my testing.
This may be a bug, will need to clarify with Opie if this is a feature we should support on Android.
Source formatting note: new line needed here to group similar functions
changed case of the backend classes to lowercase
duplicate `import re` line here?
where do we use this attribute?
I read the diff wrong, I thought it was actually adding `hasLength!Source`.

Well, then I have to nothing to say about the diff, but I'll still improve chunks on my end.
Why might echoing the string fail when it's large?
You can suppress the NewApi check for the entire library with lint.xml. It would also be more correct than your commit, since most of the methods you annotated works perfectly fine on older platforms. They "just happen to" (read: on purpose) have the same name as new APIs.
wouldn't it look nicer if you use "if ... else if ..."?
yeah I realise that, but why not wait for one more day with the one click to merge? this would have given roy the chance to update the PR, and in the meantime PG fixes would have progressed probably unimpeded.
Now we have to create/review/merge another PR later down the line, which removes probably one of the very few occurences in the core where there's cout/printf instead of ofLog.... more work overall.
Yes, I changed that see c11e027
2x '# See'
Can we move that if down to line 98 and have:

```javascript
if ( buttonElements ) {
    el.removeClass( buttonElements.bcls || "" );
} else {
    buttonInner.appendChild( buttonText );
}
```
Okay, I have change it to use a local BaseL10n object that will not fiddle with the node’s settings. It has been merged to the “next” branch.

I am also cleaning it up locally and removing quite some unused variables. Are you planning to expand the page or is it done?
Also, check out `Math.max`
the following if should just be "and'ed" into this one
change this callback to:

```php 
<?php

function (Exception $e)
{
     echo (string) $e;
}

```