Adding credentialsID node to SVN location nodes

I'm new to ruby. Recently I wrote a script to add a credentialsID node to each SVN location node in each config.xml file for all of our Jenkins jobs (We have many).

It works, but I'm guessing it could be done more elegantly. How would you fix this to be more succinct, clean, etc.

require 'nokogiri'

Dir["somepathhere/.jenkins/jobs/*/config.xml"].each do |filename|

    file = File.open(filename, 'r')
    doc = Nokogiri::XML(file)
    file.close
    locations = doc.xpath('/project/scm/locations/hudson.scm.SubversionSCM_-ModuleLocation')
    locations.each do |location|
        crednode = location.at_xpath('credentialsId')
        if crednode
            crednode.content = 'somevalue'
        else
            location.first_element_child.before("somevalue\n")
        end
    end
    File.open(filename, 'w') { |file| file.print(doc.to_xml) }
end

You can take a few shortcuts here and there, but overall it's not bad.

My comments:

-
You don't need both open and close when you just want to read everything; File.read(filename) will work just fine

-
I'd like to see some constants or command line arguments used in place of the many hardcoded strings. E.g. tag names and xpaths could be constants, while the project directory's path and credentials string could be provided from the command line.

Either one would make the script more maintainable and reusable.

-
Don't insert the new credentials tag as a raw string; use doc.create_element instead

-
The if...else isn't strictly necessary; you can just look for and remove any existing credentialsId tags, and then insert your own (though that may mess with the file's whitespace a little)

-
The Ruby convention is to use 2 spaces of indentation, not 4 (and not tabs)

My Nokogiri is a bit rusty (no pun intended1), but I imagine this should work too.

I've left the project path and the credentials string hard-coded because I don't know if you'll want those from ARGV or something.

require 'nokogiri'

CONFIG_FILE_GLOB = ".jenkins/jobs/*/config.xml"
LOCATION_XPATH = "/project/scm/locations/hudson.scm.SubversionSCM_-ModuleLocation"
CREDENTIALS_TAG = "credentialsId"

Dir["somepathhere/#{CONFIG_FILE_GLOB}"].each do |filename|
  doc = Nokogiri::XML(File.read(filename)) # simplified file read

  # create a credentials element ahead of time
  credentials = doc.create_element(CREDENTIALS_TAG, "somevalue")

  doc.xpath(LOCATION_XPATH).each do |location|
    location.xpath(CREDENTIALS_TAG).remove # remove any existing credentials
    location.add_child(credentials.clone) # insert a new one
  end

  File.write(filename, doc.to_xml) # slightly simpler file write
end

Note that in order to insert the new element properly, we must always insert a clone. If you just use add_child(credentials), and there are multiple locations, the element will end up in the last of those locations, because add_child will add or re-parent the element. So the first add_child call would add it, and subsequent ones would move it since it's the same object. Hence, the need for clone.

1) because nokogiri means hand saw

StackExchange Code Review Q#60740, answer score: 2