Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Refactor of the /searchallinsteadoftraverse option and added ablility to specific an abritrary file ordering. #80

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

icedtoast commented Sep 7, 2012

Refactor of the /searchallinsteadoftraverse logic into a FileLocator interface.

New /scriptorder option which chooses between traverse, recurse or the specified ordering locators.

Specific Ordering allows you to place a file inside the migration directory which specifies the order of the files containing in that directory and it's subdirectories. This can be used to:

  • order scripts in terms of their dependencies
  • help handle the merging of database branches.

icedtoast added some commits Sep 6, 2012

Refactor searchallinsteadoftraverse handling
Add the FileLocator interface to abstract the selection of files from
the migration folders. This replaces the need for the Migration Runner
to explicitly decide what method to call on the FileSystemAccess
interface.

 - the Traverse class replaces the get_all_file_name_strings_in method.

 - the Recurse class replace the get_all_file_name_strings_recurevly_in
   method.

The dependency injection container picks the appropiate implementation
dependening on the prescene or lack of the /searchallinsteadoftraverse
switch.
Add Specified Ordering File Locator
Add SpecifiedOrder File Locator and added a /scriptorder option which
allows you to select between the traverse, recurse or scriptorder
options. The /searchallinsteadoftraverse option also controls the
/scriptorder so RoundHouse stays backwards compatibile.

The SpecifiedOrder Locator looks at file (set by the /SpecifiedOrderFile
option, otherwise scriptorder.txt) in the migration folder and
orders each file in that migration folder and it's subdirectories based
on the position that file has in the specified order file.

Files that aren't inside the specified order file are returned after the
files that are and are sorted via the culture invariant case-insensitive
comparer on the filename relative to the migration directory.

Use Cases:

1. Using the scriptorder file allows to specify the script order in your
integration branch as you merge different feature branches into it. This
means that feature branch scripts can always rely on the existing
scripts in the integration branch being run first.

2. Stored Procedure A depends on Stored Procedure B.

Notes:

The script order file considers lines starting with # as comment lines
and are ignored.

@ferventcoder ferventcoder commented on the diff Sep 7, 2012

product/roundhouse.console/Program.cs
@@ -291,7 +292,19 @@ private static void parse_arguments_and_set_up_configuration(ConfigurationProper
option => configuration.DryRun = option != null)
.Add("searchallinsteadoftraverse=|searchallsubdirectoriesinsteadoftraverse=",
"SearchAllSubdirectoriesInsteadOfTraverse - Each Migration folder's subdirectories are traversed by default. This option pulls back scripts from the main directory and all subdirectories at once. Defaults to 'false'",
- option => configuration.SearchAllSubdirectoriesInsteadOfTraverse = option != null)
+ option => configuration.ScriptOrder = option != null ? "recurse" : "traverse")
+ .Add("scriptorder=",
+ string.Format(
+ "ScriptOrder is used to tell RoundHouse which script ordering method to use, can be: traverse, recurse or specified, defaults to: {0}",
+ ApplicationParameters.default_script_order),
+ option => configuration.ScriptOrder = option)
+ .Add("specifiedorderfile=",
@ferventcoder

ferventcoder Sep 7, 2012

Owner

ScriptOrder itself should accept a file, no need for this second argument

@icedtoast

icedtoast Sep 7, 2012

Contributor

ScriptOrder is so you can select between all the different orderings, I left /searchallinsteadoftraverse in there for backwards compatibility. ScriptOrder only accepts a fixed set of strings: traverse, recurse or specified, so that if somebody adds another ordering later they can just add a possible string to this option. (maybe it should be an enum?).

I could change /specifiedorderfile to automatically select the specified order option if it is set?

@ferventcoder

ferventcoder Sep 7, 2012

Owner

Thinking the file itself could be specified, and if it is, it automatically sets to specified.

@icedtoast

icedtoast Sep 11, 2012

Contributor

so /scriptorder will accept "traverse", "recurse", "specified" (defaulting to "scriptorder.txt") or the filename for the specified ordering?

Owner

ferventcoder commented Sep 7, 2012

I'm digging the naming convention here for traverse/recurse and the name change for the option.

Owner

ferventcoder commented Sep 7, 2012

I do like most of this commit. I like the refactoring back to one set of files.

I would like to see some automated tests coming with specified ordering. That has some complexity in it.

Contributor

icedtoast commented Sep 7, 2012

For the tests, what test runner do you use? I'm using Gallio at the moment.

Owner

ferventcoder commented Sep 7, 2012

Developwithpassion.bdd but looking to move to tinyspec

icedtoast added some commits Sep 11, 2012

Merge remote-tracking branch 'origin/master' into specified-ordering
Conflicts:
	product/roundhouse.console/Program.cs
Add tests for SpecifiedOrdering
Add Unit Tests to specify the behaviour of the specified ordering file
locator. To make it easier to test, the comparsion logic was extracted
into a Comparer implementation. The unit tests test the Comparer
implementation itself, not the SpecifiedOrdering FileLocator, which just
wraps the Comparer.

It's alright, most folks really don't get the idea of natural language in unit testing for awhile.

Change specifiedordering to ignore file extensions
Change specified ordering filelocator to ignore file extensions when it
compares files not contained within the specified order file. This means
if you have files named like this:
  - 001.sql
  - 001.env.production.sql
It will return them in that order, rather than the opposite.
Member

BiggerNoise commented Oct 9, 2017

Again, I thank you for this contribution and I suspect that I'll be incorporating these ideas as time goes on. Unfortunately, because we let this sit so long, the conflicts are going to make it difficult to merge.

That's on us, and I apologize.

@BiggerNoise BiggerNoise closed this Oct 9, 2017

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