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

Added missing extra parts into V2 Partslist #405

Merged
merged 15 commits into from
Jul 30, 2023
Merged

Conversation

BaeHenryS
Copy link
Contributor

No description provided.

Copy link
Collaborator

@Achllle Achllle left a comment

Choose a reason for hiding this comment

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

Thanks @BaeHenryS. This is not documented anywhere yet but the parts list README shouldn't be updated directly because it is autogenerated through csv_to_md.py. You either add the parts to the gobilda or digikey csv files or add them to the extra_parts.md file which gets appended to the main README. We should probably have a different file for some of the items like the DROK but for now you can cut and paste them over to the extra_parts.md file.

Once that's done, you can either run the csv_to_md.py file yourself but that isn't required because there's a Github action that runs that file and opens a PR or adds a commit to run the script.

parts_list/README.md Outdated Show resolved Hide resolved
Added the parts to extra_parts.md instead and also documented quick instruction on adding/replacing parts
Copy link
Contributor Author

@BaeHenryS BaeHenryS left a comment

Choose a reason for hiding this comment

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

Got it. Also added instructions for future people to change parts.

parts_list/README.md Outdated Show resolved Hide resolved
Co-authored-by: Achille Verheye <achille.verheye@gmail.com>
Copy link
Collaborator

@Achllle Achllle left a comment

Choose a reason for hiding this comment

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

Some minor spacing comments, then should be good to go!

Avoid committing the parts_list/README file since the action will run it and it clutters the PR

Comment on lines 250 to 251


Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep! Didn't mean to, accidentally committed on the README.md suggestion

parts_list/extra_parts.md Outdated Show resolved Hide resolved
parts_list/extra_parts.md Outdated Show resolved Hide resolved
parts_list/extra_parts.md Outdated Show resolved Hide resolved
parts_list/extra_parts.md Outdated Show resolved Hide resolved
BaeHenryS and others added 8 commits July 27, 2023 20:59
Co-authored-by: Achille Verheye <achille.verheye@gmail.com>
spacing

Co-authored-by: Achille Verheye <achille.verheye@gmail.com>
Spacing

Co-authored-by: Achille Verheye <achille.verheye@gmail.com>
@Achllle
Copy link
Collaborator

Achllle commented Jul 28, 2023

It seems like you didn't commit my suggestions. The idea is to remove the extra spacing everywhere as well as the horizontal lines which we don't have elsewhere.

@BaeHenryS
Copy link
Contributor Author

Ah, I committed but seems like I messed up after editing other parts. I have made all the spacing changes!

parts_list/extra_parts.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@Achllle Achllle left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Looking forward to more :)

@Achllle Achllle merged commit 475c24e into nasa-jpl:v2 Jul 30, 2023
1 of 2 checks passed
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.

None yet

2 participants