Skip to content
This repository has been archived by the owner on Oct 25, 2024. It is now read-only.

chore: don't override the manifest file #1509

Closed
wants to merge 3 commits into from

Conversation

lostman
Copy link
Contributor

@lostman lostman commented Dec 8, 2023

Description

Closes #1350.

Instead of overriding the Manifest file, this PR adds --override-{start-block, end-block, identifier} CLI options to forc-index {build, deploy}, which allows us to achieve what we need for the QA script without touching the manifest file.

It also looks like forc index build unnecessarily writes the module path to the manifest:

    let relative_wasm = rel_artifact_path.as_path().display().to_string();

    manifest.set_module(Module::Wasm(relative_wasm));

I think forc-index shouldn't modify the user's source files.

Testing steps

CI tests should pass.

Changelog

  • Remove the write method from the Manifest
  • forc-index build no longer overwrites the manifest file.
  • Add CLI flags to change the start and end blocks and the identifier of the indexer being deployed.
  • Change the QA script to use the new CLI flags instead of overwriting the manifest file.

@lostman lostman force-pushed the maciej/1350-dont-override-manifest-file branch from 08668ab to 8038663 Compare December 11, 2023 12:11
@lostman lostman force-pushed the maciej/1350-dont-override-manifest-file branch from 8038663 to 20aac41 Compare December 11, 2023 12:32
@lostman lostman self-assigned this Dec 11, 2023
@lostman lostman marked this pull request as ready for review December 11, 2023 16:36
@lostman lostman marked this pull request as draft December 11, 2023 18:44
@deekerno deekerno closed this Feb 6, 2024
@lostman lostman deleted the maciej/1350-dont-override-manifest-file branch March 16, 2024 05:13
@lostman lostman removed their assignment Mar 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

forc index build should preserve comments in manifest
2 participants