On 04/13/11 08:29, Virginia Wray wrote:
On 04/12/11 04:21 PM, Karen Tung wrote:
On 04/12/11 11:36, Virginia Wray wrote:
Hi --
Can I get a code review for the following bug....
http://monaco.sfbay/detail.jsf?cr=7035125
Code review is located at:
http://cr.opensolaris.org/~ginnie/7035125/
I need to get this back into the gate by the 15th, so
quick turn around would be appreciated.
thanks,
ginnie
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
Hi Ginnie,
You are using a dictionary to store the source and destination for
the profiles.
A dictionary generally have a key and a value.
In your code, you are using the "source" directory as the key, and
the "destination" directory as the value. This is very confusing to me.
I think it would be much more clear to have max of 2 items in your
dictionary.
These 2 items are optional. If people want to specify a
different source directory, they add an entry with the key "source"
and the value of the directory they want to use. Similar for
destination.
Then, the code in line 140-144 of generate_sc_profile.py can be made
much more clear, because if the "source" keyword is found, that means
ppl want to specify a different source, and you capture the value
specified..etc..
Thanks,
--karen
Hi Karen -
I'm following a format that we used in another place in the ICTs,
which Darren requested.
In that format, the dictionary key is the source file and the
dictionary value is the destination file.
Hi Ginnie,
I am not aware that other ICT checkpoints are also doing this. Now that
you mentioned it, I see that TransferFiles checkpoint also have similar
code.
IMO, the code there also would cause the same type of confusion down the
road
for somebody who is not familiar with the code.
I understand your point (in fact, I considered that as an option), but
I thought it better to keep it consistent with the other code.
I understand that you want to keep it consistent with other code.
However, I also think it is important to make the code easy to read and
maintain.
Personally, I don't see any advantage of doing it the way it is done now.
I also understand your urgency in having this change into the gate asap.
So, I think you can just push your changes the way it is now so you can
get into build 164. Then, you can file a bug and fix things so it is
more clear in all
the checkpoints before it's consumers (eg: CUD AI, CUD Text Installer)
integrates.
What do you think?
Thanks,
--Karen
Thanks,
ginnie
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
