-
-
Notifications
You must be signed in to change notification settings - Fork 156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement the contao:crawl command #985
Conversation
This needs another update for latest Escargot changes. Will work on them tomorrow. |
$this->indexer->delete($document); | ||
} | ||
} catch (IndexerException $e) { | ||
// Noop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we not want this to be logged?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about this too but I'm not sure. Right now we don't have any message either so I kept it like that for now.
All comments addressed, merged master into the PR. All green 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides the wrong class name, everything looks RTM to me.
|
||
use Terminal42\Escargot\Subscriber\SubscriberInterface; | ||
|
||
interface EscargotSubscriber extends SubscriberInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed that this does not follow our naming conventions. It should be EscargotSubscriberInterface
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, indeed! Good catch! Fixed in e591905.
e591905
to
e714b07
Compare
Thank you @Toflar. This is huge! 💪 |
This is a sub PR of #981 only containing the CLI variant. It will allow us to focus more on the technical implementation first before we have all UI/CSS changes.