Skip to content

Conversation

@ovcharenko-di
Copy link

@ovcharenko-di ovcharenko-di commented Jan 6, 2026

Зависимостей не хватало, пришлось добавить.

Возможно, для oscript 2.0 какие-то зависимости нужно удалить?

Summary by CodeRabbit

  • Chores
    • Strengthened build infrastructure by adding essential system-level components to the build environment. These additions enhance compilation capabilities, improve overall build stability, and provide better platform compatibility. The changes ensure more reliable and consistent builds across different deployment scenarios and system configurations, reducing potential infrastructure-related issues during the build and deployment process.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 6, 2026

📝 Walkthrough

Walkthrough

The Dockerfile for the oscript image has been updated to include two additional system dependencies in the package installation step: libicu-dev and pkg-config. This broadens the build environment's capabilities without modifying any runtime behavior or application logic.

Changes

Cohort / File(s) Summary
Dockerfile dependency expansion
oscript/Dockerfile
Added libicu-dev and pkg-config to the apt-get install command to extend available development libraries and utilities in the build environment

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 Two packages hop into the file so neat,
libicu and pkg-config, a build-time treat!
The Dockerfile grows, dependencies expand,
Now the oscript image has libraries on hand.

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Добавил зависимости для oscript 2.0' directly describes the main change: adding dependencies for oscript 2.0, which matches the PR's objective of adding missing dependencies.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
oscript/Dockerfile (1)

1-5: Добавьте информацию о мейнтейнере согласно guidelines.

В Dockerfile отсутствуют метки мейнтейнера. Согласно coding guidelines, следует включить информацию о мейнтейнере в Dockerfiles.

🔎 Предлагаемое добавление меток мейнтейнера

Добавьте LABEL инструкции после FROM:

 FROM ${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}${BASE_IMAGE}:${BASE_TAG}
+
+LABEL maintainer="[email protected]"
+LABEL org.opencontainers.image.title="OneScript"
+LABEL org.opencontainers.image.description="OneScript runtime with Mono"
+LABEL org.opencontainers.image.version="${ONESCRIPT_VERSION}"

Based on coding guidelines for Dockerfile*.

🧹 Nitpick comments (1)
oscript/Dockerfile (1)

43-43: Рассмотрите явное указание версии oscript 2.0.

Если PR предназначен специально для oscript 2.0, рекомендуется явно указать версию вместо использования stable, чтобы гарантировать установку нужной версии. Использование stable может привести к установке другой версии, если канал stable ещё не обновлён до 2.0.

🔎 Предлагаемое изменение для явного указания версии
-ARG ONESCRIPT_VERSION=stable
+ARG ONESCRIPT_VERSION=2.0

Или, если требуется конкретная минорная версия:

-ARG ONESCRIPT_VERSION=stable
+ARG ONESCRIPT_VERSION=2.0.0
oscript 2.0 latest version release
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 17247b5 and 84a3638.

📒 Files selected for processing (1)
  • oscript/Dockerfile
🧰 Additional context used
📓 Path-based instructions (3)
**/Dockerfile*

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/Dockerfile*: Use multi-stage builds in Dockerfiles when downloading or building dependencies
Follow ARG variable pattern for build arguments: ONEC_USERNAME, ONEC_PASSWORD, ONEC_VERSION, DOCKER_REGISTRY_URL, EDT_VERSION, COVERAGE41C_VERSION
Use registry URL pattern in base image: FROM ${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}base-image:tag
Include maintainer information labels in Dockerfiles
Combine RUN commands in Dockerfiles to minimize layers
Use build arguments for credentials (ONEC_USERNAME, ONEC_PASSWORD) instead of environment variables in Dockerfiles

Files:

  • oscript/Dockerfile
**/{Dockerfile*,build-*.{sh,bat}}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/{Dockerfile*,build-*.{sh,bat}}: Never hardcode credentials in Dockerfiles or scripts; use build arguments instead
Use container naming pattern: ${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"}onec-:${VERSION}
Tag Docker images with both specific version and 'latest' tags
Use descriptive suffixes for Docker images (-nls, -vnc, etc.) to indicate features
Include version information in Docker build outputs for debugging
Preserve error messages in Russian when originating from 1C tools in logs and output

Files:

  • oscript/Dockerfile
**/{Dockerfile*,docker-compose*.{yml,yaml}}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/{Dockerfile*,docker-compose*.{yml,yaml}}: Follow 1C standard paths for volume mounts: /opt/1cv8/, /var/1cv8/
Use consistent mount points across related Docker containers
Use standard 1C ports in Docker configurations (1540-1541 for server, 1545 for ras)

Files:

  • oscript/Dockerfile
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: firstBitMarksistskaya/onec-docker PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-18T11:03:57.697Z
Learning: Applies to **/*.{Dockerfile*,sh,bat,md,yml,yaml} : Support Russian language in documentation and code comments for 1C-specific terminology
Learnt from: CR
Repo: firstBitMarksistskaya/onec-docker PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-18T11:03:57.697Z
Learning: Applies to **/{Dockerfile*,build-*.{sh,bat}} : Use descriptive suffixes for Docker images (-nls, -vnc, etc.) to indicate features
Learnt from: CR
Repo: firstBitMarksistskaya/onec-docker PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-18T11:03:57.697Z
Learning: Applies to **/README* : Include build examples for new Docker images in README
Learnt from: CR
Repo: firstBitMarksistskaya/onec-docker PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-18T11:03:57.697Z
Learning: Applies to **/Dockerfile* : Use multi-stage builds in Dockerfiles when downloading or building dependencies
📚 Learning: 2025-12-18T10:14:58.871Z
Learnt from: ovcharenko-di
Repo: firstBitMarksistskaya/onec-docker PR: 85
File: crs-apache/Dockerfile:4-4
Timestamp: 2025-12-18T10:14:58.871Z
Learning: In Dockerfiles that use BuildKit (ENABLE_BUILDKIT=1 or DOCKER_BUILDKIT=1 in the environment), the parameter expansion syntax ${DOCKER_REGISTRY_URL:+"$DOCKER_REGISTRY_URL/"} in FROM instructions is valid. Hadolint DL1000 may flag this as a false positive; if you rely on BuildKit features, you can safely ignore that report. Ensure you are building with BuildKit enabled and that the Docker version supports this syntax (Docker 18.09+).

Applied to files:

  • oscript/Dockerfile
🔇 Additional comments (1)
oscript/Dockerfile (1)

16-17: Dependencies libicu-dev and pkg-config are correctly required for oscript 2.0.

These packages are necessary: libicu-dev provides .NET globalization support on Linux, and pkg-config is required for native library discovery during builds. The additions are appropriate.

@nixel2007 nixel2007 merged commit c06573e into firstBitMarksistskaya:feature/first-bit Jan 6, 2026
11 of 12 checks passed
@nixel2007
Copy link
Member

Спасибо!

@ovcharenko-di для 2.0 по идее можно снести весь моно после запуска ovm

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants