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

feat: Add station_id field to measurements storage and publishing #249

Merged
merged 7 commits into from
Apr 23, 2024

Conversation

PatrickNercessian
Copy link
Contributor

@PatrickNercessian PatrickNercessian commented Apr 12, 2024

DRAFT, DO NOT MERGE

Links:
filecoin-station/roadmap#96

@juliangruber juliangruber marked this pull request as draft April 13, 2024 19:06
Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

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

Looks mostly good! 👏🏻

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
test/test.js Outdated Show resolved Hide resolved
@PatrickNercessian PatrickNercessian changed the title Draft: Add station_id field to measurements storage and publishing Feat: Add station_id field to measurements storage and publishing Apr 15, 2024
Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

Feel free to open another pull request to implement the validation rule accepting stationId values of a given string length only.

@bajtos bajtos changed the title Feat: Add station_id field to measurements storage and publishing feat: Add station_id field to measurements storage and publishing Apr 17, 2024
@bajtos
Copy link
Member

bajtos commented Apr 17, 2024

@PatrickNercessian What's the status of this pull request? Is there anything left to improve or is it good to be landed?

@bajtos
Copy link
Member

bajtos commented Apr 17, 2024

You will also need to update the branch to bring the commits that landed on the main branch since you started this work. The simplest option is to click the Update branch button at the bottom of this page. Just remember to run git pull in your local clone before you make any further changes!

@PatrickNercessian
Copy link
Contributor Author

@bajtos I was going to do new PR for stationId constraints as you mentioned, but noticed there were some other code churn updates that could be reduced as we did in other PRs, so I thought I'd combine both.

Let me know your thoughts.

@PatrickNercessian PatrickNercessian marked this pull request as ready for review April 17, 2024 22:05
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@juliangruber juliangruber enabled auto-merge (squash) April 22, 2024 14:23
@bajtos
Copy link
Member

bajtos commented Apr 22, 2024

The required CI checks will run after we land #254

@juliangruber juliangruber merged commit dcd9db2 into filecoin-station:main Apr 23, 2024
8 checks passed
@PatrickNercessian PatrickNercessian deleted the station_id branch April 30, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants