Comment 5 for bug 2050227

Revision history for this message
Jason Stephenson (jstephenson) wrote (last edit ):

I had a look at this again this morning, and I actually don't think the removal of the required fields on those IDL entries is wrong. When you review my criteria for a field being required, then these fields are not required:

Required fields are that those that come from a database table, have a "NOT NULL" constraint in the schema, and do not have a default value assigned in the database.

"[C]ome from a database table," means literally that. Readonly entries based on views don't need to have readonly set since they cannot be updated.

Many of the examples that changed are from readonly IDL entries, or the database has a default value.

All of that being said, my process skipped fields that already had the readonly attribute set and skipped readonly IDL entries. If they had these attributes before, they should not have changed. It also bothers me that these fields changed during the whitespace clean up. I am not sure what happened there. As I mentioned previously I worked on this over several days, I used a couple of branches and had to rebase it a couple of times as other commits made changes to the IDL. I also did a few of these steps more than once.

I am not happy with how things have turned out, so I'll start this over in another branch.