Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.I propose that we remove all the features I added, and that are not strictly necessary. This would help in maintaining the code, and in finding bugs in the code.
If we agree on this, and on what we should remove, I will take care of doing this task.
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | xmlsitemap_user_install_092789.patch | 2.09 KB | avpaderno |
| #11 | xmlsitemap_user_094578.patch | 5.89 KB | avpaderno |
| #7 | xmlsitemap_user.module.txt | 11.89 KB | avpaderno |
Comments
Comment #1
avpadernoAs first item to remove, I would propose the Rules file integration.
I will create a new project that will give Rules support to XML sitemap.
Comment #2
avpadernoAnother candidate could be the code that changes the sitemap priority of some links basing on some conditions.
As there is already a way to allow third-party modules to change the priority of any links that go in the sitemap, that part can be removed from the project code, and placed in a module that is not part of this project at all.
Comment #3
Anonymous (not verified) commentedScope creep for 6.x-1.0-BETA. We have confirmed reports of people having success with the current code. Let's not fix what ain't broke.
Comment #4
Anonymous (not verified) commentedKiam, I thought we agreed to peer review. I don't see a patch in this issue yet you've implemented #2. Please, lets just fix bugs so we can get this sucker out of the door. No more feature creep before the BETA please.
Comment #5
avpadernoIf I would know what a creep is, I could reply (I am joking :-)).
Actually, I simply removed the features I implemented, and of what I am the only responsible, as there have not been reviews about the real necessity of adding such features (to say the truth, webchick, and Dave expressed their negative opinion).
I would rather cut the code before releasing a beta, but if you see any reasons to postpone the task, then we will do so. Frankly, I don't see how to carry code that is not strictly needed could help on making a beta release.
Comment #6
Anonymous (not verified) commentedI'm not opposed to removing the features before the RC but we really need to get a freeze on to get out a BETA. Then we can discuss the pieces that need to be changed. Offering a patch to apply for review helps the others follow along with those changes even if you decide to commit the change. It also gives an easier means to reverse the patch if necessary.
What else do you think should be removed? Lets discuss it, either here or there. I prefer an issue for each piece so we can track them separately like I've done at #453146: xmlsitemap-6.x-1.0-rc1 Ready, Set, Go. The format gives a nice report to those interested.
Comment #7
avpadernoOK, I try again (the comment I added has been mysteriously blanked).
This is how xmlsitemap_user.module would appear.
I removed the code that changes the priority, and removed the implementation of the bulk operations; like I said, they will be implemented from a new module that is not part of this project.
Comment #8
avpadernoI agree. I am changing the report to be specific for a module.
Comment #9
avpadernoI also report that I added a new function to xmlsitemap.module (
xmlsitemap_api()), which simply returns a string useful for third-party modules to verify the current version of the project code.I added this function because the modules don't need to know the exact version of the module; it's a similar function that is present in Views modules, with the difference that it's the modules to check for the API version exposed by the project code.
Comment #10
Anonymous (not verified) commentedComment #11
avpadernoHere comes the patch, updated to the latest commits done (it has been hard to start from the beginning for three times :-)).
Comment #12
Anonymous (not verified) commentedThe patch file should be created from the root of the the module directory.
cd mydev/modules/xmlsitemap
cvs diff -up xmlsitemap_user > xmlsitemap_user.patch
Anyway the patch doesn't break http://sample.com/sitemap.xml so it can be committed.
Comment #13
Anonymous (not verified) commentedWait, I didn't test what I thought I was testing.
Comment #14
Anonymous (not verified) commentedNow I'm happy.
Comment #15
avpadernoThe changes have been committed in CVS; now it's time for the .install file.
Comment #16
avpadernoHere comes the... patch.
Comment #17
avpadernoComment #18
Anonymous (not verified) commentedVisually this looks good. Does anyone have a method to test the upgrade path? My 5.x have already been upgraded. I don't want to install just to check it but if I must ...
Comment #19
avpadernoThe changes have been committed in CVS.
Comment #21
avpadernoThis task needs to be re-rolled again.
Comment #22
Anonymous (not verified) commentedComment #23
dave reidIf we haven't gotten to this yet, I doubt we'll ever get to it. Marking as won't fix.