silinternational / ssp-base

Base Docker image for simpleSAMLphp
3 stars 1 forks source link

define variables needed in the header template #247

Closed briskt closed 2 months ago

briskt commented 2 months ago

Fixed

Note: It may be possible to reduce this code duplication by defining our own Template class, inherited from SimpleSAML\XHTML\Template, but I think that even that would have to be duplicated once for each module. Perhaps it could be extracted to our ssp-utilities repo.

sonarcloud[bot] commented 2 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
14.3% Duplication on New Code

See analysis details on SonarCloud

mtompset commented 2 months ago

_Note: It may be possible to reduce this code duplication by defining our own Template class, inherited from SimpleSAML\XHTML\Template,

Exactly what I thought after seeing all these tweaks.

but I think that even that would have to be duplicated once for each module.

Duplicated how? As far as I can tell, the use line would change, that's it.

Perhaps it could be extracted to our ssp-utilities repo.

So, instead of changing the use line, add a functionality to do the tweaks on the template object? I think the extended object idea is better.

briskt commented 2 months ago

_Note: It may be possible to reduce this code duplication by defining our own Template class, inherited from SimpleSAML\XHTML\Template,

Exactly what I thought after seeing all these tweaks.

but I think that even that would have to be duplicated once for each module.

Duplicated how? As far as I can tell, the use line would change, that's it.

Perhaps it could be extracted to our ssp-utilities repo.

So, instead of changing the use line, add a functionality to do the tweaks on the template object? I think the extended object idea is better.

There's a hard boundary between modules, such that you can't call code from one module to the other. It may be possible to do it in this repo, but outside of a module, but my PHP isn't strong enough to think of all the required details. I'd welcome anyone to step in and clean this up for me.

briskt commented 2 months ago

The advantage of the extended object idea is that you don't accidentally forget to add these two lines everywhere. I'll approved, but it isn't the cleanest way. Maintenance headaches await.

I note you said "I'll approve" but yet you didn't. Change of heart?

briskt commented 2 months ago

The advantage of the extended object idea is that you don't accidentally forget to add these two lines everywhere. I'll approved, but it isn't the cleanest way. Maintenance headaches await.

I note you said "I'll approve" but yet you didn't. Change of heart?

Doh, my mistake. You did.