-
Notifications
You must be signed in to change notification settings - Fork 42
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
feat: new build pipeline #544
base: master
Are you sure you want to change the base?
Conversation
a1a3034
to
38e9e3b
Compare
5824d22
to
119b7f2
Compare
@@ -12,21 +12,21 @@ type Options = MarkdownItPluginOpts & { | |||
singlePage: boolean; | |||
}; | |||
|
|||
const collect = (input: string, options: Options) => { | |||
const collect = async (input: string, options: Options) => { |
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.
Это рушит обратную совместимость
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.
Данная функция используется только в рамках cli, а значит можно проставить как async исключительно collect функции, но при этом требуется обновить cli под новый функционал. Это необходимо сделать в любом случае для перехода на async await. Самое сложное будет обновить index функции под преобразования html.
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.
collect является частью трансформ пайплайна
Чтобы сделать асинхронный collect нужно поддержать асинхронный transform
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.
Для этого есть ПР в cli
diplodoc-platform/cli#866
@@ -29,6 +29,38 @@ export type Heading = { | |||
items?: Heading[]; | |||
}; | |||
|
|||
export interface FsContext { |
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.
В другом ПР оставлял комментарий по поводу ожидаемого здесь интерфейса
|
||
export interface DependencyContext { | ||
resetDeps?(path: string): void; | ||
markDep?(path: string, dependencyPath: string, type?: string): void; |
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.
В ПР я вижу использование только метода markDep
зачем остальные?
зачем markDep опциональный?
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.
markDep опциональный чтобы обеспечить обратную совместимость и не ломать lint'инг. Все функции могут быть пустыми, так как они внешние для уменьшения риска, что в какой то среде данная функция будет не указана.
src/transform/typings.ts
Outdated
}; | ||
} | ||
|
||
export interface RevisionContext { |
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.
Этот контекст где-то используется в ПР?
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.
В Cli используется, но заранее лучше проставить тут, чтобы можно было разрабатывать плагины на основе ревизии (добавление версионности, истории и прочее)
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.
С точки зрения transform это неиспользуемый код. Ему тут не место.
No description provided.