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

Repo setup #10

Merged
merged 4 commits into from
Nov 2, 2024
Merged

Conversation

SchrodingersStruggle
Copy link
Collaborator

Initial setup for repository

  • Added a devcontainer, updated requirements.txt, and added linting to repository.

Contents

  • .devcontainer/python/devcontainer.json - Devcontainer to set up python in repository.
  • .github/workflows/pylinttest.yml - Allows linting in PR's
  • requirements.txt - Edited existing file, added numpy, pandas, matplotlib, and pylint.

Label:

  • Added organization label to characterize this pull request.

Reviewer

Added standard devcontainer seen in our previous homeworks.
Added essential items to requirements.txt
Used pylint as I (personally) find it nicer to work with than Flake8.
Got the file from the midterm repository.
Copy link
Contributor

@laserlab laserlab left a comment

Choose a reason for hiding this comment

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

Looks good,

check for my comment about splitting the linting / testing

Copy link
Contributor

Choose a reason for hiding this comment

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

In the last Homework we had that separated. You can talk to @kylemasc917 and @HeshamElsaman if that was more convenient. I would merge it like this for now though.

Choose a reason for hiding this comment

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

Not separating them would be better, not 100% sure, though.
I'd suggest not separating them and adding comments to the .yaml file..

@SchrodingersStruggle
Copy link
Collaborator Author

@kylemasc917 If I were to separate the linting and pytest, would I need to clear the cache of pylint like in flake 8, or would that not be necessary?

@kylemasc917
Copy link
Contributor

Not that I know of, but you'll have to test it out and see.

@SchrodingersStruggle
Copy link
Collaborator Author

Okay I will keep them merged for now, as suggested by @laserlab, and merge this request so people can work with linting in the repository.

@SchrodingersStruggle SchrodingersStruggle merged commit e12e16b into ubsuny:main Nov 2, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants