cryogen-project / cryogen-core

Cryogen's core
Eclipse Public License 1.0
69 stars 62 forks source link

Allow certain paths to be excluded from the sitemap #150

Closed seancorfield closed 3 years ago

seancorfield commented 3 years ago

The compiler now passes the whole config map into sitemap/generate and that destructures ignored-files and a new optional key ignored-paths and filters the latter out of the result of the find-assets call.

The ignore-path function mirrors the ignore-name function in cryogen-core.io -- should it perhaps be in that ns in case other code might want to reuse it later?

Is the new config key :ignore-paths a good choice? Should it be something sitemap-specific (since only sitemap/generate uses it)?

holyjak commented 3 years ago

I think that a sitemap specific name might be better. The existing :ignored-files is actually used everywhere during the compilation process so its names is fitting. Perhaps :sitemap/ignore-paths?

seancorfield commented 3 years ago

It would be the first (only) qualified keyword in the config, but I agree that something sitemap-specific is a better name. Either :sitemap-ignored-paths or :sitemap/ignored-paths seem reasonable (ignored rather than ignore to match the existing terminology).

holyjak commented 3 years ago

Good point!

On Thu, 29 Oct 2020, 17:52 Sean Corfield, notifications@github.com wrote:

It would be the first (only) qualified keyword in the config, but I agree that something sitemap-specific is a better name. Either :sitemap-ignored-paths or :sitemap/ignored-paths seem reasonable (ignored rather than ignore to match the existing terminology).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/cryogen-project/cryogen-core/pull/150#issuecomment-718883984, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEYSPWWHIX34AYNC4UPOOLSNGMTPANCNFSM4TCZMTRA .

seancorfield commented 3 years ago

PR updated:

bombaywalla commented 3 years ago

I've reviewed this PR. Looks good to me. Will merge.