Add UUIDs to resources and validations #100

Merged
merged 2 commits into from Mar 1, 2013

Conversation

Projects
None yet
3 participants
@gfontenot
Contributor

gfontenot commented Jan 25, 2013

I know this is seriously going to break the way the Learn website sees the validations, but the (upcoming) iOS app really needs a way to uniquely identify the validations and the resources.

Side note: the html-css trail seems to have both resources and references. I don't believe this is intentional.

@djcp

This comment has been minimized.

Show comment Hide comment
@djcp

djcp Jan 25, 2013

Contributor

It would be useful to be able to refer items in trails authoritatively.

Contributor

djcp commented Jan 25, 2013

It would be useful to be able to refer items in trails authoritatively.

@adarsh

This comment has been minimized.

Show comment Hide comment
@adarsh

adarsh Feb 10, 2013

Contributor

@gfontenot Apologize for the delay on this PR.

Do you mind rebasing against master and resubmitting?

Travis (now working) is failing the commit. link

Contributor

adarsh commented Feb 10, 2013

@gfontenot Apologize for the delay on this PR.

Do you mind rebasing against master and resubmitting?

Travis (now working) is failing the commit. link

@gfontenot

This comment has been minimized.

Show comment Hide comment
@gfontenot

gfontenot Feb 10, 2013

Contributor

@adarshpandit No worries. I ended up deleting the original commit and just re-running the script to generate the UUIDs. Seemed easier than dealing with the merge conflicts, and I wanted to make sure everything had an ID.

Contributor

gfontenot commented Feb 10, 2013

@adarshpandit No worries. I ended up deleting the original commit and just re-running the script to generate the UUIDs. Seemed easier than dealing with the merge conflicts, and I wanted to make sure everything had an ID.

@adarsh

This comment has been minimized.

Show comment Hide comment
@adarsh

adarsh Feb 24, 2013

Contributor

@gfontenot - This looks good to squash and merge

Contributor

adarsh commented Feb 24, 2013

@gfontenot - This looks good to squash and merge

@gfontenot

This comment has been minimized.

Show comment Hide comment
@gfontenot

gfontenot Feb 25, 2013

Contributor

@adarshpandit Would it be better for me to just regenerate the UUIDs against master then merge? Otherwise, we need to be sure to add UUIDs for the Git Branching tutorial and the RubyMotion video. Your call. I'd be happy to regenerate the UUIDs and push again to this PR.

Contributor

gfontenot commented Feb 25, 2013

@adarshpandit Would it be better for me to just regenerate the UUIDs against master then merge? Otherwise, we need to be sure to add UUIDs for the Git Branching tutorial and the RubyMotion video. Your call. I'd be happy to regenerate the UUIDs and push again to this PR.

@adarsh

This comment has been minimized.

Show comment Hide comment
@adarsh

adarsh Feb 26, 2013

Contributor

Go ahead and regenerate since I merged some new stuff.

If you're using a script, can you commit it and add instructions to the
README on how to use it (should others need to)?

Thanks!

On Mon, Feb 25, 2013 at 10:11 AM, Gordon Fontenot
notifications@github.comwrote:

@adarshpandit https://github.com/adarshpandit Would it be better for me
to just regenerate the UUIDs against master then merge? Otherwise, we need
to be sure to add UUIDs for the Git Branching tutorial and the RubyMotion
video. Your call. I'd be happy to regenerate the UUIDs and push again to
this PR.


Reply to this email directly or view it on GitHubhttps://github.com/thoughtbot/trail-map/pull/100#issuecomment-14060728.

@adarshp http://twitter.com/adarshp
developer for thoughtbot, inc http://www.thoughtbot.com
313.454.1515 (m)

Scheduling with me? See bit.ly/adarshcalendar

Contributor

adarsh commented Feb 26, 2013

Go ahead and regenerate since I merged some new stuff.

If you're using a script, can you commit it and add instructions to the
README on how to use it (should others need to)?

Thanks!

On Mon, Feb 25, 2013 at 10:11 AM, Gordon Fontenot
notifications@github.comwrote:

@adarshpandit https://github.com/adarshpandit Would it be better for me
to just regenerate the UUIDs against master then merge? Otherwise, we need
to be sure to add UUIDs for the Git Branching tutorial and the RubyMotion
video. Your call. I'd be happy to regenerate the UUIDs and push again to
this PR.


Reply to this email directly or view it on GitHubhttps://github.com/thoughtbot/trail-map/pull/100#issuecomment-14060728.

@adarshp http://twitter.com/adarshp
developer for thoughtbot, inc http://www.thoughtbot.com
313.454.1515 (m)

Scheduling with me? See bit.ly/adarshcalendar

@gfontenot

This comment has been minimized.

Show comment Hide comment
@gfontenot

gfontenot Feb 27, 2013

Contributor

Actually, yeah. It could probably be useful as a rake task. I'll add it there. People should only need to run it to generate UUIDs if they are adding a new resource/reference/validation

Contributor

gfontenot commented Feb 27, 2013

Actually, yeah. It could probably be useful as a rake task. I'll add it there. People should only need to run it to generate UUIDs if they are adding a new resource/reference/validation

Add UUIDs to trails
* Also turns validations into dictionary objects, instead of a simple
array.
* Add UUIDs to valid json fixture
@gfontenot

This comment has been minimized.

Show comment Hide comment
@gfontenot

gfontenot Mar 1, 2013

Contributor

@adarshpandit I think this should (finally) be ready. I wasn't sure how to test the UUID generator class. If you have any ideas, I'd be happy to add the tests.

Contributor

gfontenot commented Mar 1, 2013

@adarshpandit I think this should (finally) be ready. I wasn't sure how to test the UUID generator class. If you have any ideas, I'd be happy to add the tests.

@gfontenot

This comment has been minimized.

Show comment Hide comment
@gfontenot

gfontenot Mar 1, 2013

Contributor

I should note that validate:uris fails, but it's also failing on master.

Contributor

gfontenot commented Mar 1, 2013

I should note that validate:uris fails, but it's also failing on master.

Rakefile
@@ -28,4 +31,11 @@ namespace :validate do
end
end
+namespace :generate do
+ desc 'Generate UUIDs'
+ task :uuid => :environment do

This comment has been minimized.

Show comment Hide comment
@adarsh

adarsh Mar 1, 2013

Contributor

Style: Prefer new hash syntax

@adarsh

adarsh Mar 1, 2013

Contributor

Style: Prefer new hash syntax

This comment has been minimized.

Show comment Hide comment
@gfontenot

gfontenot Mar 1, 2013

Contributor

Should I update the other tasks as well?

@gfontenot

gfontenot Mar 1, 2013

Contributor

Should I update the other tasks as well?

This comment has been minimized.

Show comment Hide comment
@adarsh

adarsh Mar 1, 2013

Contributor

Might as well. It's out of scope for this PR but always good to be
consistent.

On Fri, Mar 1, 2013 at 11:03 AM, Gordon Fontenot
notifications@github.comwrote:

In Rakefile:

@@ -28,4 +31,11 @@ namespace :validate do
end
end

+namespace :generate do

  • desc 'Generate UUIDs'
  • task :uuid => :environment do

Should I update the other tasks as well?


Reply to this email directly or view it on GitHubhttps://github.com/thoughtbot/trail-map/pull/100/files#r3209071
.

@adarshp http://twitter.com/adarshp
developer for thoughtbot, inc http://www.thoughtbot.com
313.454.1515 (m)

Scheduling with me? See bit.ly/adarshcalendar

@adarsh

adarsh Mar 1, 2013

Contributor

Might as well. It's out of scope for this PR but always good to be
consistent.

On Fri, Mar 1, 2013 at 11:03 AM, Gordon Fontenot
notifications@github.comwrote:

In Rakefile:

@@ -28,4 +31,11 @@ namespace :validate do
end
end

+namespace :generate do

  • desc 'Generate UUIDs'
  • task :uuid => :environment do

Should I update the other tasks as well?


Reply to this email directly or view it on GitHubhttps://github.com/thoughtbot/trail-map/pull/100/files#r3209071
.

@adarshp http://twitter.com/adarshp
developer for thoughtbot, inc http://www.thoughtbot.com
313.454.1515 (m)

Scheduling with me? See bit.ly/adarshcalendar

This comment has been minimized.

Show comment Hide comment
@gfontenot

gfontenot Mar 1, 2013

Contributor

👍

Will do.

@gfontenot

gfontenot Mar 1, 2013

Contributor

👍

Will do.

lib/helpers/uuid_generator.rb
+class UUIDGenerator
+ def initialize(file_name)
+ @file_name = file_name
+ @trail = JSON.parse(File.read(@file_name))

This comment has been minimized.

Show comment Hide comment
@adarsh

adarsh Mar 1, 2013

Contributor

I might do this in the run method, since it's actually doing stuff instead of just initializing. Should file loading or parsing fail, it would seem weird to have that happen when I run a new.

@adarsh

adarsh Mar 1, 2013

Contributor

I might do this in the run method, since it's actually doing stuff instead of just initializing. Should file loading or parsing fail, it would seem weird to have that happen when I run a new.

This comment has been minimized.

Show comment Hide comment
@gfontenot

gfontenot Mar 1, 2013

Contributor

Good point. Will move.

@gfontenot

gfontenot Mar 1, 2013

Contributor

Good point. Will move.

@adarsh

This comment has been minimized.

Show comment Hide comment
@adarsh

adarsh Mar 1, 2013

Contributor

@gfontenot Yeah I noticed the master failing as well.

Minor comments - looks good to merge.

Contributor

adarsh commented Mar 1, 2013

@gfontenot Yeah I noticed the master failing as well.

Minor comments - looks good to merge.

Add UUID generation rake task
* Will assign UUIDs to any item that doesn't already have one.
* Can be run from generate:uuid
* Converted hashes to 1.9 syntax

@gfontenot gfontenot merged commit cc2cebe into master Mar 1, 2013

1 check failed

default The Travis build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment