Skip to content
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

various: remove old/stale/unused dependencies #3188

Merged
merged 11 commits into from
Jan 26, 2024
Merged

Conversation

Fang-
Copy link
Member

@Fang- Fang- commented Jan 19, 2024

We remove all references to and dependencies on graph-store, metadata-store, group-store, and versions of channel types older than the previous generation.

This does mean that we remove migration/import logic for versions of groups preceding the previous one, but the migration path coming from that far back was already known to need some manual guidance.

Notably inclusion is a bugfix in /app/notify: it was still scrying into %group-store, when it should have been updated to scry into %groups instead. PTAL.

Marking as draft for now, because:

  • I should test a clean install off of this (plus base-dev and landscape-dev folders).
  • I should cull old marks, make sure all the remaining ones still build. (At the very least, there's still marks for old update types.)
  • We may want to hold off on release until we've helped out known stragglers stuck in the graph-store era (there's at least one).

Fixes LAND-1243.

We remove all references to and dependencies on graph-store,
metadata-store, group-store, and versions of channel types older
than the previous generation.

This does mean that we remove migration/import logic for versions of
groups preceding the previous one, but the migration path coming from
that far back was already known to need some manual guidance.

Notably inclusion is a bugfix in /app/notify: it was still scrying into
%group-store, when it should have been updated to scry into %groups
instead.
@Fang- Fang- added ops/meta Operational (CI/CD, etc.) or meta level issues devex labels Jan 19, 2024
Copy link

linear bot commented Jan 19, 2024

This sort of suggests it's stale...
/sur/hark-store no longer exists, here or in tloncorp/landscape. The
type used here also doesn't nest inside of the type used by current
%hark. This suggests the code here is stale, but for now we simply make
it compile in the absence of the desired dependency.
@Fang-
Copy link
Member Author

Fang- commented Jan 23, 2024

Both the groups and talk desks can now be constructed from the following copy operations:

cp -RL urbit/pkg/base-dev/* ship/desk/
cp -RL landscape/desk-dev/* ship/desk/
cp -RL landscape-apps/desk/* ship/desk/

(Replacing desk/* at the end there with talk/* for the talk desk, obviously.)

Neither of them need all the files that the dev desks provide, but given we don't have a way of specifying/copying the exact subset we need, we just stay with the status quo of "superfluous inclusions" for now.

We should, when we next deploy, probably do a full mounted desk wipe + the above copy operation, to make sure we're at least not leaving any of the now-removed files in place. Perhaps CI's deploy logic already takes care of this?

@Fang- Fang- requested a review from arthyn January 23, 2024 20:47
Copy link
Member

@arthyn arthyn left a comment

Choose a reason for hiding this comment

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

lgtm

desk/app/notify.hoon Outdated Show resolved Hide resolved
desk/sur/notify.hoon Show resolved Hide resolved
@arthyn arthyn marked this pull request as ready for review January 26, 2024 18:32
@arthyn arthyn merged commit e8c11b4 into develop Jan 26, 2024
1 check passed
@arthyn arthyn deleted the m/dependency-cleanup branch January 26, 2024 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devex ops/meta Operational (CI/CD, etc.) or meta level issues
Projects
None yet
2 participants