Skip to content

Conversation

@cadenmyers13
Copy link
Contributor

No description provided.

@cadenmyers13
Copy link
Contributor Author

@sbillinge sorry, meant to message earlier. Here's the PR with the .github directory.

Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

please see inline comments

- name: check out diffpy.pdfmorph
uses: actions/checkout@v3
with:
repository: diffpy/diffpy.pdfmorph
Copy link
Contributor

Choose a reason for hiding this comment

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

please make sure the read and try and understand what these are doing....you may guess that this won't work in the new repo after just copying it over....

@cadenmyers13
Copy link
Contributor Author

@sbillinge whoops... just did this briefly without checking. Should be good now. I'll get working on the except conditions in flake8.

@sbillinge sbillinge merged commit d8e81a6 into diffpy:cookie Jul 25, 2024
@sbillinge
Copy link
Contributor

super, thanks for this!

I recommend to get in the mode of, as much as possible, trying to understand what everything is doing as you are doing it, This is a great way to learn. You don't have to understand everything as it slows things down too much, but there is a concept of "code monkey" which is someone who just copy-pastes code without even trying to understand why and how it works, but this person will progress much more slowly..... I am not saying you are a code monkey,I'm just giving you permission to take initiative...

@cadenmyers13 cadenmyers13 deleted the workflows branch July 25, 2024 05:53
@cadenmyers13
Copy link
Contributor Author

cadenmyers13 commented Jul 25, 2024

super, thanks for this!

I recommend to get in the mode of, as much as possible, trying to understand what everything is doing as you are doing it, This is a great way to learn. You don't have to understand everything as it slows things down too much, but there is a concept of "code monkey" which is someone who just copy-pastes code without even trying to understand why and how it works, but this person will progress much more slowly..... I am not saying you are a code monkey,I'm just giving you permission to take initiative...

Will do! I usually try my best to understand things before I make/push edits but sometimes it slips through the cracks 😂 .

I've merged (locally) all the flake8 branches and this workflow branch into one scrap branch to run pre-commit and everything is passing except check yaml due to an error in conda-recipe/meta.yaml. I tried to look into this error a while ago on a different package but I couldn't quite figure out a way to get around this. Is this a problem that needs to be fixed? I can definitely look again.

Also, I will look at the bare except errors flake8 was throwing to see if I can get a more specific fix besides Exception.

@sbillinge
Copy link
Contributor

Fantastic!!!

flake8 has a hard time with jinja templates. Is it failing on a line that looks like {{ something.something }} something else? We may have to update to have the code ignore that, but I htink in general we are not keeping the conda-recipes in with the code. We are basically using a cookiecutter to build the conda meta.yamls. So we don't need to move over that conda-recipes folder at all, though please do a comparison of the meta.yaml in the staged-recipes-cookiecutter and htis one to see if we need to add anything to the latter. You can open a discussion on slack with the cookie group.

@sbillinge
Copy link
Contributor

I will be on trains today, so not sure how well, if at all, the internet will work, but if you can start making those PRs of the flake8ing I can try and start merging them. Nice idea to make a big branch to see if you can ge them all to pass!

@cadenmyers13
Copy link
Contributor Author

Fantastic!!!

flake8 has a hard time with jinja templates. Is it failing on a line that looks like {{ something.something }} something else? We may have to update to have the code ignore that, but I htink in general we are not keeping the conda-recipes in with the code. We are basically using a cookiecutter to build the conda meta.yamls. So we don't need to move over that conda-recipes folder at all, though please do a comparison of the meta.yaml in the staged-recipes-cookiecutter and htis one to see if we need to add anything to the latter. You can open a discussion on slack with the cookie group.

Yeah that is where I'm getting the error. I'll do all the comparison tomorrow and ask the group if I need help. I have 4 PRs open related to flake8 errors right now so I think those are ready for review when you get the chance!

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.

2 participants