Skip to content
This repository was archived by the owner on Mar 12, 2019. It is now read-only.

Conversation

@kakwok
Copy link
Collaborator

@kakwok kakwok commented Jan 22, 2017

For #110

  • Only CfgCVSBasePath and MasterSnippetList(ie the file name of grandMaster) should remain in LV1's userXML.
  • All other userXML accesses will refer to the "grandMaster" file
  • Putting a bunch of userXML accesses in EventHandler complicates things because LV2s have empty userXML. Shall we move those out to LV1EventHandler?

@kakwok kakwok requested a review from sethcooper January 25, 2017 10:55
Copy link
Collaborator

@sethcooper sethcooper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Shouldn't we go into error if the CfgCVSBasePath or the MasterSnippet list can't be found (i.e., on catching an exception)? I don't really like relying on defaults, nor on just sticking a warning message in the log. Of which there are many already. Speaking of which...
  • Please cut down on logger.info and logger.warn
  • I don't really understand what the return "" are doing in the getHCALuserXMLelementContent and getNamedUserXMLelementAttributeValue functions
  • Otherwise, assuming it's been tested, it seems fine

@kakwok
Copy link
Collaborator Author

kakwok commented Jan 25, 2017

getHCALuserXMLelementContent and getNamedUserXMLelementAttributeValue has been re-directed to look for the grandMaster. If the file name of the grandMaster is missing, the two functions return ""

@kakwok
Copy link
Collaborator Author

kakwok commented Jan 25, 2017

-Removed the unused special snippets filenames from userXML, e.g. #139
-Handles no MasterSnippetList exception
-Move userXML from EventHandler to LV1EventHandler
-Add a switch to getHCALuserXMLelementContent and getNamedUserXMLelementAttributeValue so that they can access real userXML.

@kakwok kakwok mentioned this pull request Jan 26, 2017
@kakwok
Copy link
Collaborator Author

kakwok commented Jan 26, 2017

Also closing #28

@kakwok kakwok merged commit e699b1b into HCALRunControl:fixMaster Jan 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants