labofsahil/github-event-dataset-2012 huggingface dataset:
@Senior-Student многие повторяются, но это ничего, так и должно быть. А вот с рекурсией не видел вроде, да и не нужна она здесь, если честно... Единственное место, которое теоретически можно вогнать в рекурсию - это смена значения ячейки, и всё. Если делать рекурсию для всего метода mutate - то очень легко выйти за рамки стека, и будет "ой".
Для сравнения значений строк в NSString существет специальный метод isEqualToString, используйте его вместо !=
Object#tap doesn't exist in Ruby 1.8 I think, no idea how compatible anything else in here is though.
Why's that?

I know we don't use it, but I could see the strong/weak dance being abhorrent to some people.
Honestly, I kind of want to get rid of `sequenceMany:`. I know it mirrors `>>`, but it seems unnecessarily confusing, and `flattenMap:`/`sequenceNext:` can cover all the same use cases.
yeah, looks like pyDev did that.. gotta go fix that. sorry
I mean JSONObjectWithData changed the options type from NSJSONReadingOptions to ZincJSONReadingOptions, but dataWithJSONObject swapped WritingOptions for ReadingOptions.
we should change Raw s.t. _preload_data() sets _preloaded  to True (otherwise the data can be preloaded but _preloaded is False, which doesn't make sense)
You don't have to do that. the BaseContextSensitiveTest that this class extends is annotated as @Transactional, so it is rolled back at the end of the @Test-annotated method.
At the start of each @Test-annotated method you should have an in-memory database that is the combination of what's in standardTestDataset.xml and anything created in any @Before-annotated method.
@parndt done.
Ha, good catch. I meants to try different servers but failed. I will fix it and add the tests I've been meaning to add.
oups ;) I haven't had time to use it finally so I didn't spot it
Thx
It's part of the Coding Standards that Symfony follows (PSR and [there own one](http://symfony.com/doc/current/contributing/code/standards.html))
You should only use the class name and not the qualified namespace. Every good PHP documentor (i.e. phpdocumentor2 and sami) will match the `use` statements with the classnames here.
Doesn't this come to the same thing? But the latter is probably better anyways (e.g. if we have a blacklist of things we won't parse0?
Error handling? 1.) rpcinfo/rpcbind might not be installed 2.) might be broken / return erroneous output. Maybe this is not that important as "no servers found" is not vitally important.
Don't free `pkey` and `cert` here if `PKCS12_parse()` returned an error. That function allocates memory for the key and the certificate and updates the pointers in the argument list. On error, it frees the memory but doesn't set the pointers to NULL. You'd be double-freeing them here.
Sorry. Must have screwed up merge.
You should add a `This option is available since version 1.1.11.` like it is there for the data attribute.
Creo que hay un `Assert.ThrowsException` o algo así. Igual está bien.
Que nabo, es cierto, ¿lo implementará bien Raven? mañana lo pruebo.
Ah, this one comes back to me a lot. I try to catch them. Want me to push a clean up on this or leave it to the larger cleanup job?
Maybe :) Everyone follows his style. But Github recommends doing so https://github.com/styleguide/ruby/. In this particular case may be obvious whether a value is assigned to a variable or two variables are compared. You decide.
Yeah, each of these models should probably have their own limits. Doesn't make a lot of sense for them to have the same arbitrary default.
I'll agree the linebreak was goofy
Yes, the paths output by libgit2 diff are all relative to the (possibly non-existent) working directory of the repository.
I'd certainly *like* to make this recommendation, but I think agree with @pelson, that it might be discouraging for simple submissions.  How about we just say something like "tox makes it easy to determine if your working copy introduced any regressions before submitting a pull request."
Based on my earlier comment, this method could really do with a docstring (whilst your there, ```get_supported_filetypes()``` and ```get_supported_filetypes_grouped()``` should get a docstring too, if possible).
To follow convention, either do "import matplotlib.gridspec as mgridspec" or "from matplotlib.gridspec import GridSpec" (and make appropriate changes below).
pedantic: don't do bare except clauses.

Is there any risk of 2to3 getting patched for this and causing problems?
It would, but I guess another question is are we overriding based on slave key name or on the general slave sub-array?
me and @special discussed about the reasons and decided to keep the 2-phases as currently lacking a valid alternative

For more details on how the doublezoom is performed (comment #5):
https://bugs.nemomobile.org/show_bug.cgi?id=404
I'm not quite sure why those changes belong to this pull request?
Theoretically OGC parameter values are case-sensitive, so we should use uppercase WPS here
colocar comentário em cima da classe
e " classe necessária para aplicar efeitos hover na foto do membro"
This can probably be removed right now, it feels like this kind of stuff can easily be left in place and there is no need for that.
This shouldn't be enabled by default.
@christianv Missing 2 spaces for indenting.
@arnodeclercq Can you change `0px` to `0`?
why that? wouldn't it be better to use the number of threads shogun uses, i.e., ```parallel->get_num_threads()```
"A GridField is..."
Naomi can't win: first Sean told her off for missing spaces (around "."s) and then Ingo tells her off for adding spaces (around "->"s). :-P

It might be worth making a separate "when to insert spaces" section on http://doc.silverstripe.org/sapphire/en/trunk/misc/coding-conventions ?
Usually we just add a .1 here... so a newer version would overwrite our patched one.
Shouldn't __session_commit/rollback_transaction and __session_checkpoint check WT_CONN_TRANSACTIONAL?   I guess they'll eventually fail, but seems simpler to test at the top layer.

In begin-txn, I would test WT_CONN_TRANSACTIONAL before "are there open cursors", I think we want the "you aren't configured for txns" error to take precedence over the failure because cursors are open.

The code in __session_begin_transaction looks wrong to me (there's a __wt_errx that doesn't jump to the err label).  I think you want:

```
        if (!F_ISSET(S2C(session), WT_CONN_TRANSACTIONAL))
                WT_ERR_MSG(session,
                    EINVAL, "Database not configured for transactions");
        if (TAILQ_FIRST(&session->cursors) != NULL)
                WT_ERR_MSG(session, EINVAL, "Not permitted with open cursors");
```

It doesn't appear to me it's possible to have open cursors on files that aren't participating in the transaction.  That was a feature of Berkeley DB that I know got used -- the idea is there are temporary data sources that don't have anything to do with transactions (and are presumably re-created from scratch after a crash).   Alternatively, in WT, since everything in a cursor, what happens if you have an open statistics cursor when you start a transaction -- that would fail?  It's bad for txn and non-txn operations to mix on a table, but I think it's OK to mix txn and non-txn data sources.
Possibly better wording?
> Disabling an extension retains its files and settings but removes any functionality added by the extension.
Is it possible to define a more specific return type; maybe using a union like ``null|Object``, or ``string|array``?
Minor, just be consistent.  use all scale factors or exact values.  Also the last line should have been scaleX instead of 2
Returning the value even if it's not a `Plucky::Query` instance is consistent with ActiveRecord. Alternatively, I'd be okay with raising an exception, but I think that a `NoMethodError` is not the appropriate type of exception.
I would have preferred you to use || instead of or, but it's just a small detail ;)  Thanks a lot fo this fix.
This return is not used
If we wan't this to quack like a hash, shouldn't we implement `delete` rather than having a magic value *mean* delete? I'm not really excited about the idea of calling a setter that could possibly have the side effect of deleting something.
if should be lowercase
This @return is not consistent with the above documentation. Returns multiple regexes. The top portion does not mention anything about nulls.
Am 16.04.2012 20:46, schrieb Stefan Krastanov:
>> @@ -42,5 +41,9 @@ for language in languages:
>>           print "    Processing '%s'" % template
>>           name = os.path.splitext(template)[0]
>>           s = t.render({name + "_active": "active"})
>> -        s = smart_str(s) + "\n"
>> -        open(os.path.join(language, template), "w").write(s)
>> +        f = open(os.path.join(language, template), "wb")
>> +        try:
>> +          f.write(s.encode("utf-8"))
>> +          f.write("\n")
>
> I mean if it is an ascii character, then how are we mixing it with utf-8?

UTF-8 has been carefully designed so that all valid 7-bit ASCII 
character streams are also valid UTF-8 character streams.

Unicode is 21 bits.
UTF-8 is a variable-length encoding:
0.0000.0000.0000.0xxx.xxxx -> 0xxx.xxxx.
0.0000.0000.0xxx.xxxx.xxxx -> 110x.xxxx 10xx.xxxx
0.0000.xxxx.xxxx.xxxx.xxxx -> 1110.xxxx 10xx.xxxx 10xx.xxxx
x.xxxx.xxxx.xxxx.xxxx.xxxx -> 1111.0xxx 10xx.xxxx 10xx.xxxx 10xx.xxxx
(Longer encodings are possible in theory but not expected to materialize 
in the foreseeable future.)

IOW any 7-bit ASCII character has the same encoding in UTF-8 and in 
ASCII, and it is possible to freely concatenate valid UTF-8 and 7-bit 
ASCII strings.
Aha I get it, so the way it is now is OK?
There is also one problem: now some other templates (change password, account) are without a label on a button - most probably - I cannot check because I get the error when I try to open my profile. Should I fix this anyway?
Can you order those the same as they are in Facebook backend?
Move this maybe to main docstring of this backend. You can also create some initial description (like, "Foursquare user fields are:").
(Like `station_id`.)
The else isn't really needed since it's implicit - there is only one way to go if the if is not entered. Not a deal breaker though =)
This worked for me. Perhaps you forgot to run `make`?
Oh, I see. They're using libtool in the head builds. Cute.
Почему не из сервис менеджера?
equal can be used here
Only sense of EnterCombat is the DoScriptText, for which I don't have data in DB, so I commented it, because it's an useless overriding method in this case. As I already said: If you have the data tell me and I'll re-implement it.
So on further thought, this should have a test, but I think we should leave the added filter in.
i see. makes perfect sense, thanks.
well, removing them is not a big deal: they are private so they will be removed if they are not used.

But this reference should indeed be wrapped in a check
Shouldn't RuntimeException be thrown ?
Why don't you use human-readable array names, instead of the rather esoteric one letter identification ?
It is possible empty record is a valid record, say if we were storing lines from a file. But for Protobufs and Thrift, it is never valid.
The timestamp collision issue @kimballa pointed out for the key/value meta data restore is also present here.
Shouldn't this be done on RoleEnvironment.Changed instead of Changing?
s/CSS Path/Selector
@bnoordhuis 
> *BSD or OS X users?

Getting mod_pagespeed to build on *BSD or OSX will be a project in its own.  Probably not too bad, in that people have done it in the past, but there's no testing on them so supporting code has probably rotted.
Right, I suspected as much. The GNU implementation of std::string is an Abomination unto $DEITY (there's a number of performance gotcha's) but I guess it's alright for now.
This wasn’t a stylistic change. I could be wrong, but I thought I saw some false-positives (or, uh, false negatives?) from the original syntax. I could try changing it back now that other things have been fixed if you want.
Can you document the new parameters above?
Unfortunately, this doesn't work in Ruby 1.9. Specs are failing with this error:

```
1) TwitterCldr::Formatters::TimespanFormatter#format#ago works for a variety of units
   Failure/Error: @formatter.format(-3273932, :year).should == 'Vor 0 Jahren'
     expected: "Vor 0 Jahren"
          got: "[\"Vor \", \"0\", \" Jahren\"]" (using ==)
   # ./spec/formatters/calendars/timespan_formatter_spec.rb:19:in `block (4 levels) in <top (required)>'
```

The problem is that `Array#to_s` works differently in 1.8 and 1.9. In 1.8 it simply joins elements in one string, that doesn't really make sense in most of the cases. That's why it was changed in 1.9 and now it return smth more similar to `Array#inspect`. Same for `Hash`. So, if you want to join array into a string you should use [`Array#join`](http://www.ruby-doc.org/core-1.9.3/Array.html#method-i-join) instead. And, of course, you can use `Enumerable#map` as @camertron suggested.
Why not to add link to forge to README?
Have you checked how important this optimization is?  I'm guessing not, given the later comment on line 110.  But also, I can imagine someone wanting to test for very subtle differences between poisson and gaussian noise and being surprised by this behavior subverting their tests.  So I'd suggest that Poisson should really mean Poisson until we have reason to believe that a speedup is necessary.
I added this feature.  `output.file_name` has a default `ext` of '.fits' (if using type=NumberedFile).  Likewise `file_name` for `weight`, `psf` and `bad_pix`.  `input.catalog` has a default `ext` of '.asc'.  Any other use of NumberedFile (such as `dir` in this demo) have a default `ext` of None (or '').
Change does not match the docstring, which gives a slew of DOxygen grumbles.  See below!
This is a breaking API change, previously 'val' could be passed an object of properties beyond just 'id' and the full instance could be instantiated directly. This change will result in a structure like id: { ... } which is not compatible. If only a string id is going to be passed it should be checked to not be an object and not null, in general I'd prefer to make it such that an object is always passed unless there is a good reason to only take an ID as a parameter. The object form must work to support current statement structure.
Agreed


On Nov 6, 2012, at 8:19 PM, Jason Bau <notifications@github.com> wrote:

> In main/templates/exams/view_exam.html:
> 
> > @@ -34,22 +34,30 @@
> >  {% block addl_scripts %}
> >  <script>
> >      $(document).ready(function () {
> > +
> > +    var SUBMISSION_LIST_URL = "{% url 'courses.exams.views.view_my_submissions' common_page_data.course.prefix common_page_data.course.suffix exam.slug %}";
> > +
> > +    // Add button that just takes you back to submission list without submitting form again
> > +    var returnToListBtn = document.createElement('input');
> > +    $(returnToListBtn).attr('type', 'button');
> > +    $(returnToListBtn).attr('id', 'returnToListBtn');
> > +    $(returnToListBtn).attr('title', 'Click to return to the submissions list without re-submitting form');
> > +    $(returnToListBtn).val('Return Without Submitting');
> > +    $(returnToListBtn).addClass('btn btn-spaced');
> > +    $(returnToListBtn).click(function (ev) {
> > +        document.location = SUBMISSION_LIST_URL;
> > +    });
> > +    
> > +    var submitBtn = $('input[type|="submit"]').not('#login_button');
> I think going forward we should provide our own submit button instead of having that come out of the database HTML.
> 
> —
> Reply to this email directly or view it on GitHub.
>
This solution, with a separate transition, certainly works. Is there a reason, though, not to simply enable the search button *every* time the "appear" event is fired (i.e., including the first time)?
"thrashing" rather than "trashing", presumably.
Please pull from the google repo instead of code.jquery.com
why? you'll have a bug for this for small screens cases then.
remote-actor-status-check?
We shouldn't need to set the CURLOPT_POST constant.  The method is being set a few lines above with the CURLOPT_CUSTOMREQUEST constant.
you're missing some spaces here to be PEP8 compliant, If I'm not wrong
You're making this variable but only ever use it once. Perhaps just stick `location.search` in the `if ` statement.
Good question.  These are the "notfications" that were introduced recently.  At the moment, the dispatcher doesn't send notifications but that could be changed.  We could make it so that notifications have templates.

I'll create an issue to review Oscar's communications and possible consolidate them (https://github.com/tangentlabs/django-oscar/issues/319)
Not yet. Only in the query string.

On 29. 09. 12, at 11:37, Mitar <notifications@github.com> wrote:

> In datastream/api.py:
> 
> >          _order = -1
> > +        _key = 's10'
> Could at least keys be 10s? Do we use them anywhere as JavaScript or Python object attribute value? So object.10s or something?
> 
> —
> Reply to this email directly or view it on GitHub.
> 
>
yes. Again, it was there as a reminder, as no string should ever be assigned to 'keyManagement' (which is an array) but that’s how the API works at this very moment… :-/

I’ll make a proper TODO comment instead.
tables is a reference to a dictionary. no need to return it.
too bad this has to be redeclared here...
Should we wait until Crubier's stuff is merged in? Peabody124's idea was that we could update all the UAVOs at once, and that would be a perfect time to review comments and formatting. (And decide if UAVOs will us a tab or spaces for indentation.)
А ввод из stdin не подерживается?
Вместо пробела правильнее использовать \s
We're following a convention of always having exactly one newline at end - not sure if that's important to back-end as well.
Nice. Hey @jefflembeck - look good to you?



On Oct 8, 2012, at 4:41 AM, Markus Staab wrote:

> In quickconcat.php:
> 
> >  // Guess file type
> > -$fext = preg_match( '/\.(js|html|css)$/', $files[ 0 ], $match );
> > +$fext = preg_match( '/\.(js|html|css)$/', $files[ $idx ], $match );
> could use array_shift($files) instead
> 
> —
> Reply to this email directly or view it on GitHub.
> 
>
It would be very useful to me if you could explain what makes it so. I generally work in a vacuum, but want to be a good OS contributor.
Super minor detail--breaking this onto a second line will make it a bit easier to read.
php sdk was ported from Java SDK, I guess php's encode logic could have already been included in path function.
I think that's better to change the name/folder to something like helloworld-jdg to no colide with previous existing helloworld quickstart
I would like to put an explicit wait (WebDriverWait) here for the 'Profile updated' message to appear at the top of the page.

Without it I think the test could possibly jump to the assertions before the save changes has been completed.
That was written before the Berlin event.
did you actually try to build this on linux?
forgot to turn on auto-fill (I'm on emacs) - fixed
missing space "cosine"| -> "cosine" |
typo: postive > positive
you could use sklearn.utils.shuffle
This is a larger context for the change:

                    if (fd->leastAsSpecialized(fd2) || fd2->leastAsSpecialized(fd))
                    {
                        if (!global.params.useDeprecated)
                        {
                            TypeFunction *tf = (TypeFunction *)fd->type;
                            if (tf->ty == Tfunction)
                                deprecation("use of %s%s hidden by %s is deprecated", fd->toPrettyChars(),
                                      Parameter::argsTypesToChars(tf->parameters, tf->varargs), toChars());
                            else
                                deprecation("use of %s hidden by %s is deprecated", fd->toPrettyChars(), toChars());
                        }
                        s = rtlsym[RTLSYM_DHIDDENFUNC];
                        break;
                    }

I guess it does, originally I didn't remove it because I thought there was more logic inside the if, maybe with side effects, but I guess TypeFunction *tf = (TypeFunction *)fd->type; can be executed even when useDeprecated is false without problems.
owner_ should be the 2nd parameter, if only for consistancy with other similar handler such as NativeListView and NativeSlider that i'm modifying to have the pointer to owner as well
Should verify that arg[0] is indeed an int before converting to integer.  If it is not ToInteger will always return 0.  This should at least be logged.

Just add a qDebug() <<
with the class name::function and something like "received arg is not an integer"
With great power, comes great responsibility, and I am not a responsible vim user.
This should be inside the addLoop()
This is still relying on the class layout (of StaticBlock being the first thing in it AND no pre-padding added to the class). Maybe getting the address of 'length' is a better approach.
Is event an alternative to the eclserver, or only in addition?  The use of | suggests the former, but the help text from the command suggests the latter.
Just wondering, is it possible to validate the signed-off-by: <user> matches the git username here?
I think that needs to be `{{ hit.signature|urlencode }}`
Please use an i18n message here instead of a static string. Also, specifying the exception type makes it easier to reason about. `ArgumentError` is appropriate. E.g.:

```ruby
raise ArgumentError, t('unroutable_resource') unless route
```

Make sure `Dispatcher` includes the `Webmachine::Translation` module.
Если не секрет, зачем такие сложные названия для баз? Почему не что-нибудь более "лёгкое", например:
```yaml
development:
  adapter: sqlite3
  database: blog_dev
#...
test:
  adapter: sqlite3
  database: blog_test
#...
holy_blog:
  adapter: sqlite3
  database: holy_blog
#...
```
Oh, not sure why but somehow i missed that. Had to restart the patch due to massive merge issues yesterday. Will fix it.
The easiest way to convert to boolean is `!!this._map`.
Sorry, yeah, it wasn't that clear. 

It would be great if you could modify the test as follows: 

- (temporarily) change line 50/64 back (no cast, only command as parameter).  

        ExecutionResults kresults = ksession.execute( this.command );
- modify the test so that it _fails_ because the context is not passed. 
- change line 50/64 back to what it is now (cast, command _and_ context as parameter). 

This is to make sure that if someone else sees this code later and thinks "Huh, that doesn' t make sense! Let's get rid of the cast and context parameter!", that the test then immediately breaks and they realize why that needs to be done. 

Thank you for your patience, Mauricio!
@ozten - Strangely enough, I tried reading the cookie value in Android 3.3 browser with cookies disabled.  The value comes back.  Whack.
Zend_View? isn't this a PhpRenderer?
I completely misunderstood the behaviour of apply. I have added the feature with the new updates.
Isn't that what the line just below does for us? I mean isn't `config.temp_folder` set to the system's tmp folder that the system cleans up?
Should this be returning a boolean rather than an int?
@shacker Could you fix this by using 2 spaces everywhere?
you should fix it as it is not valid phpdoc
fixed in next commit.
I could certainly change the print_status to attempting to create service if that is more to your liking
Typo in ```@git_branch```
Having this at the end of the file is a little weird. Why not move it into spec/support?
This should better be removed ([as in other exs](https://groups.google.com/d/topic/cesium-dev/d1BReGxbkuo/discussion)).
Perhaps it is just personal preference, but I wouldn't reuse this for two different shaders; it's not like we are setting this, and then doing something generic with it later.  I'd just have separate ones for the color and pick passes.  Then when JavaScript introduces the `let` keyword, we'll have block-level scope and be happy.
`camera.position` is a `Cartesian3` and a `w` property would be added to it. Is it faster to do that than cloning after the multiply?
Should we validate width and height so we don't divide by zero?
I understand the check but it doesn't change the fact that a generic base class should not be aware of it's children.  If parity is to support persistent only for AlertDialog then we should do that.  If creating a new TiUIAlertDialog is needed then so be it.
Not sure what prongs are. I think it might be simpler to just say, "The color of the activity indicator."

If I understand correctly, I think both this and indicatorDiameter should have a description that says something like:

Overrides the color/size specified by the [style](Titanium.UI.ActivityIndicator.style) property.
Definition or link of fill behavior.
I also fix the java typo
Это в какой такой системе координат? И почему это по умолчанию?

http://www.gorissen.info/Pierre/maps/googleMapLocation.php
I guess this should be var
Please use empty lines to group the code blocks
they are from the analysis code, which I moved out. I'll move out these functions as well. Well spotted, Thanks!
Please follow proper formatting. Should be tabs, not spaces.
ditto.
LGTM
space after if (as you did above)
since this is new code it should use local functions instead of goto
I can make it take a range. Removing the ability to pass `char` arrays probably won't cause too much grief to anyone.
fix naming convention throughout
Just spotted this: statements of the form `assert_equal true, condition` would be simpler and clearer as `assert condition`.

For consistency, it probably makes most sense to fix this throughout the rest of the gem as well.
This one looks quite weird. We'd better not to use this.evented to pass the returned result.
This doesn't have a target product.
`require_relative 'swf_login'`
It doesn't.  I started down the "ask the server helper for its port" path, and the tendrils run deep enough that it's not worth fixing.  (A lot of the test code is super brittle :()
pls add javadoc for what this constant should be used
why not using saveDescription(TransformationDescription... description); instead of a list and a single entry?
Dropped the first one and fixed the second.
Turns out it was unnecessary, so I deleted the entire file.
Do you want us to do this for all the the getTitle's in that file?
Looking at the devel docs, this does make sense.  Disregard that comment.
Come to think of it, this idea of loading only the current relations might have been a brain fart :)
In fact we're using the default java setting with only a minor difference, the import order is the same as Eclipse and we need to change the method alignment to match the one in eclipse too.

We should share that file somewhere, maybe in ArcBees-tools in a config folder?
inconsistent spaces after commas
When set as an attr_accessible:

root@admin:/opt/dell/crowbar_framework# rails c
You did not specify how you would like Rails to report deprecation notices for your development environment, please set config.active_support.deprecation to :log at config/environments/development.rb
Loading development environment (Rails 3.2.6)
irb(main):001:0> r = CmdbRun.new(:name => 'my test', :type => 'flatulant')
=> #<CmdbRun id: nil, name: "my test", description: "t", type: "flatulant", order: "10000", cmdb_id: nil, cmdb_map_id: nil, created_at: nil, updated_at: nil>
irb(main):002:0> r.to_json
=> "{\"type\":\"flatulant\",\"updated_at\":null,\"created_at\":null,\"description\":\"t\",\"name\":\"my test\",\"id\":null,\"order\":\"10000\"}"
irb(main):003:0> r.type
=> "flatulant"
irb(main):004:0> r.class
=> CmdbRun(id: integer, name: string, description: string, type: string, order: string, cmdb_id: integer, cmdb_map_id: integer, created_at: datetime, updated_at: datetime)
irb(main):005:0>

When NOT set as an attr_accessible (and with rake db:dump;rake db:migrate)
“Cannot mass assign”


From: Rob Hirschfeld [mailto:notifications@github.com]
Sent: Wednesday, November 21, 2012 3:36 PM
To: dellcloudedge/barclamp-crowbar
Cc: Maltin, Judd
Subject: Re: [barclamp-crowbar] CmdbRun and CmdbEvent basics, with unit tests [1/1] (#279)


In crowbar_framework/app/models/cmdb_run.rb:

> @@ -13,11 +13,42 @@

>  # limitations under the License.

>  #

>  class CmdbRun < ActiveRecord::Base

> -  attr_accessible :title, :body

> +  attr_accessible :name, :description, :order, :type

I'm not sure that type can be accessible - it's a build in override.

—
Reply to this email directly or view it on GitHub<https://github.com/dellcloudedge/barclamp-crowbar/pull/279/files#r2199087>.
I think you are correct for Arnold, but for that to be safe in general, we'd need to positively specify that RendererServices::get_attribute() should have no side effects.
This class is not autoloadable using a PSR-0 autoloader. If you would like it to be loadable, it should be in the sub-path "JMS/SerializerBundle/Annotation/MapFields.php".
Changes in this file should adhere to the style guide
It doesn't get picked up when placed inside of ui.css.
I don't see how this can work - the Signal.cpp file still has an ifndef WIN32 in there so none of the functions is actually implemented but they are now still called from everywhere...
Just assert about the suite.
Makes sense - let's add a javadoc comment on the old store indicating it will move to store in 3.0

Also, these new packages all new package-info.java
Gary, thanks a lot!
So, what I see here:
1. @DirtiesContext is OK for test class with one test method. We definitely need to clean up all the resources after its execution. Or do it manualy before test or after. Otherwise we catch issues with other tests. E.g. Oleg and me indicate here, that in-memory Derby doesn't destroyed properly before other tests's start. 
2. Don't use  @DirtiesContext in the tests with several methods: it decreases execution time and makes tests more understandable in regard to used resources.

So, I'm going to investigate what Gunnar did and will learn something new ;-)
Fixed (changed the commit in this branch).
The value must be a service id
Do we really need to make this a parameterized class when we have access to $foreman_proxy::nameservers inside proxydhcp anyway?
Ok, I removed them.
Thought I'd give myself a little credit (-:
test simple pass through to view on success
You should quote and escape this string.
why these lines are commented out? 
If they are not needed please remove, or enable them
Nitpick:

`userwith2dbs`
`userwith3db`

don't go holding this PR up for this though
Instead of creating variables and only using them to populate an array that you'll access by number, why not just put an object into your questionsArray and then access the named properties of that object in your later code? That way, there's no confusion about what thing is in which array slot.
Hmm, it's weird that tests are passing then O_o
I'm fixing that.
юзай простіші логіни
ми говорили, що тест реєстрації можна винести в core. а тут достатньо залогінитись юзером з фікстур і зайти перевірити лист
that's perfect.
I wasn't sure that outputDir will take over the targetPath, and I didn't have time to check it without the targetPath,
I will test it later this week.
Arabic-speaking contributors just confirmed the calendar layout has to be switched to RTL in their language (first day = rightmost column, last day = leftmost column). I’d suggest using lists, possibly nested lists, instead of a table here.
jamesladd, note for you
why is there a ChatNotifier and a MessageNotificationHandler, and why are they both different. Can you explain the role of each if you think they should be different?
:) Not tempting enough!
Can you add a comment along these lines to the function, please:
Are we synchronized as far back in time as we are able to synchronize?

I was going to propose renaming the function to something like "isFullySynchronized", but for the changes for some of the Andreas-requested speedups, the directionality is important, and DawnOfTime arguably does have some upsides over "syncedAsFarBackAsWeCanGo".
Override this method if the order in which properties are set is significant.
When I let the <?> inside, the wsdl plugin gives an error, because he couldn''t convert from ...reflection to Class
This blank line should be removed IMHO.
Since you probably favor Markdown (README.md) to provide documentation - Should we remove all docbook references/files?
Такой-то баг был. Видимо никто не удаляет посты с доски, раз никто его не словил.
Awesome that we were able to get rid of all this
I think you really need the Apache 2.0 boilerplate here.
maybe i lack context, but should we not still allow options to be overwritten? or was this $options parameter something that was not used before?