MagLev / maglev

GemStone Maglev Ruby Repository
http://maglev.github.io
517 stars 41 forks source link

Exposes backup and restore methods on Maglev::Respository #423

Closed johnnyt closed 8 years ago

jc00ke commented 9 years ago

Thoughts on specs for these kinds of methods?

AllenOtis commented 9 years ago

To demonstrate functional backup/restore commit some persistent ruby data. backup stop stone replace the extent dbf file with a copy from the $GEMSTONE/bin restart the stone bootstrap the stone run the ruby restore method(s) check that persistent data is present.

alternatively restart stone run restore with $GEMSTONE/bin/topaz -l, using SystemRepository restoreFromBackup: now run maglev-ruby without bootstrapping and rubykernel methods and the persistent data should be present

On Sat, Jul 25, 2015 at 11:30 PM, Jesse Cooke notifications@github.com wrote:

Thoughts on specs for these kinds of methods?

— Reply to this email directly or view it on GitHub https://github.com/MagLev/maglev/pull/423#issuecomment-124950339.

AllenOtis commented 9 years ago

Also, in terms of documentation comments in the kernel .rb file, they could be lifted from the comments in the corresponding Smalltalk methods.

On Sat, Jul 25, 2015 at 11:30 PM, Jesse Cooke notifications@github.com wrote:

Thoughts on specs for these kinds of methods?

— Reply to this email directly or view it on GitHub https://github.com/MagLev/maglev/pull/423#issuecomment-124950339.

johnnyt commented 9 years ago

Any other thoughts on this? - I'd like to get this merged into master.

@jc00ke - do you think we needs specs before its merged?

jc00ke commented 9 years ago

I'd prefer to see specs for this (an integration test would probably prove handy)

johnnyt commented 9 years ago

Here's another question

Right now src/kernel/bootstrap/ObjectSpace1.rb defines the following (so these are always available):

module ObjectSpace
  Finalizer = Object.__resolve_smalltalk_global(:RubyFinalizer)
  System = Object.__resolve_smalltalk_global(:System)
  Repository = Object.__resolve_smalltalk_global(:Repository)
  SystemRepository = __resolve_smalltalk_global(:SystemRepository)
end

Then lib/ruby/1.9/maglev/repository.rb defines the following (so you must run require "maglev/repository" before they are available):

module Maglev
  Repository = __resolve_smalltalk_global(:Repository)

  class Repository
    # Returns the current repository (equivalent to the Smalltalk global
    # SystemRepository).
    def self.instance
      __resolve_smalltalk_global(:SystemRepository)
    end
  ...
  end
  ...
end

What are people's thoughts on where the SystemRepository stuff should live (constants as well as code)?

timfel commented 9 years ago

I'd prefer to have all of these on Maglev::Repository. They are custom to Maglev, and people just notice that when using it.

timfel commented 9 years ago

As for specs, yes, those would be good, otoh if it is too hard to test correctly, I'd be happy to skip testing it in this specific case, because we are actually just running the Smalltalk code which does have tests on the GemStone side.

jc00ke commented 9 years ago

Is there some other way we could be testing these exposed methods? Feels wrong to just be exposing them with no test coverage, but maybe that's just me.

johnnyt commented 8 years ago

@jc00ke - I'd like to merge this in - how are you feeling now about skipping the tests for backup/restore and rely on the Smalltalk code/tests?