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.

Sign up for GitHub

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

Jump to bottom

BCF-2685: rm plugins/cmd/chainlink-solana #10973

Merged
merged 1 commit into from
Oct 23, 2023
Merged

BCF-2685: rm plugins/cmd/chainlink-solana #10973

merged 1 commit into from
Oct 23, 2023

Conversation

jmank88
Copy link
Contributor

@jmank88 jmank88 commented Oct 16, 2023 •
edited
Loading

@jmank88 jmank88 mentioned this pull request Oct 16, 2023
Merged
@github-actions
Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

@jmank88 jmank88 mentioned this pull request Oct 16, 2023
Merged
@jmank88 jmank88 changed the title rm plugins/cmd/chainlink-solana BCF-2685: rm plugins/cmd/chainlink-solana Oct 16, 2023
@jmank88 jmank88 force-pushed the BCF-2685-solana-loopp-extraction branch from 8636767 to e0189cb Compare October 16, 2023 20:25
patrickhuie19
patrickhuie19 previously approved these changes Oct 16, 2023
View reviewed changes
@jmank88 jmank88 force-pushed the BCF-2685-solana-loopp-extraction branch 11 times, most recently from 71482fd to 5b3cac5 Compare October 19, 2023 12:54
@jmank88 jmank88 mentioned this pull request Oct 19, 2023
Merged
1 task
@jmank88 jmank88 force-pushed the BCF-2685-solana-loopp-extraction branch 4 times, most recently from 3492195 to 78a1d7e Compare October 20, 2023 21:20
patrickhuie19
patrickhuie19 previously approved these changes Oct 20, 2023
View reviewed changes
@jmank88 jmank88 force-pushed the BCF-2685-solana-loopp-extraction branch 3 times, most recently from 54e526a to 643ce5a Compare October 21, 2023 23:03
krehermann
krehermann reviewed Oct 22, 2023
View reviewed changes
core/services/checkable.go
// Name returns the fully qualified name of the component. Usually the logger name.
Name() string
}
// Deprecated: use services.HealthReporter
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: i understand the intention of this comment, but it's confused because this package is also services. how about listing the fully qualified relayer package name in the comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm but this generates a godoc link since we are actually importing it as services. Should we alias it?

@jmank88 jmank88 force-pushed the BCF-2685-solana-loopp-extraction branch from 643ce5a to b79fd07 Compare October 23, 2023 13:39
@jmank88 jmank88 marked this pull request as ready for review October 23, 2023 13:39
@jmank88 jmank88 requested review from a team, jkongie, samsondav, archseer and cfal as code owners October 23, 2023 13:39
@jmank88 jmank88 force-pushed the BCF-2685-solana-loopp-extraction branch from b79fd07 to 93fdf8c Compare October 23, 2023 13:50
@cl-sonarqube-production
Copy link

SonarQube Quality Gate

Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

76.2% 76.2% Coverage
0.0% 0.0% Duplication

aalu1418
aalu1418 approved these changes Oct 23, 2023
View reviewed changes
plugins/chainlink.Dockerfile
Comment on lines +23 to +31
RUN go list -m -f "{{.Dir}}" github.com/smartcontractkit/chainlink-solana | xargs -I % ln -s % /chainlink-solana

# Build image: Solana Plugin
FROM golang:1.21-bullseye as buildsol
RUN go version
WORKDIR /chainlink-solana

COPY --from=buildgo /chainlink-solana .
RUN go install ./pkg/solana/cmd/chainlink-solana
Copy link
Contributor

Choose a reason for hiding this comment

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

nit - is this resusable somehow? seems like building starknet/other chains would probably use something similar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the short term, it can be reused as one build image for all the plugins.
It is just a work around for go install being incompatible with replaces in the go.mod. Longer term, we don't want to be installing from source anyways, so this complication goes away.

@jmank88 jmank88 added this pull request to the merge queue Oct 23, 2023
Merged via the queue into develop with commit ff84f9f Oct 23, 2023
93 checks passed
@jmank88 jmank88 deleted the BCF-2685-solana-loopp-extraction branch October 23, 2023 15:12
@snehaagni snehaagni mentioned this pull request Nov 14, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aalu1418 aalu1418 aalu1418 approved these changes

@patrickhuie19 patrickhuie19 patrickhuie19 approved these changes

@jkongie jkongie Awaiting requested review from jkongie

@samsondav samsondav Awaiting requested review from samsondav

@archseer archseer Awaiting requested review from archseer

@cfal cfal Awaiting requested review from cfal

@krehermann krehermann Awaiting requested review from krehermann

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jmank88 @krehermann @patrickhuie19 @aalu1418