Skip to content

Conversation

waaghals
Copy link
Contributor

@waaghals waaghals commented Sep 9, 2021

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets n/a

I've used events to generate a robots.txt. This makes for an easy extension point for dynamically modifing the robots.txt.
I'm having doubts about the working of Kunstmaan\SeoBundle\EventListener\FileRobotsTxtListener.
It no longer uses the request's basepath. But I do not understand how it could have worked previously, as the controller would never be called when the file exists in te public directory.

@ProfessorKuma ProfessorKuma added this to the 5.9.0 milestone Sep 9, 2021
Copy link

@ProfessorKuma ProfessorKuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @, your PR needs some changes

  • This PR seems to need a milestone of a minor release.

Copy link

@ProfessorKuma ProfessorKuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @, your PR passed all our requirements.

Thank you for contributing!

@acrobat acrobat assigned acrobat and unassigned acrobat Sep 11, 2021
@acrobat acrobat self-requested a review September 11, 2021 18:59
@acrobat acrobat modified the milestones: 5.9.0, 5.10.0 Oct 10, 2021
Copy link
Member

@acrobat acrobat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a specific reason every action/check for robots file/content is moved to an event listener? I would think it would enough to keep the current behaviour and just dispatch an event at the end that would allow you to hook into the robots content and modify it or event fully replace it.

Is there a specific need for this setup?

@acrobat acrobat removed this from the 5.10.0 milestone Oct 12, 2021
@waaghals
Copy link
Contributor Author

My reasoning was to allow for easier changing of the logic.
For example, lets say I would like the content from the file robots.txt to be appended to the existing content from the entity
instead of acting as a fallback.

By making smaller listeners this could easily be done by only replacing FileRobotsTxtListener. Otherwise the new implementation would also need to implement the existing logic of the Entity fetching etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants