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

style: update billinge email #247

Merged
merged 2 commits into from
Jan 8, 2025
Merged

Conversation

cadenmyers13
Copy link
Contributor

No description provided.

Copy link

codecov bot commented Jan 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 52.63%. Comparing base (c7f7b8c) to head (1d27a4b).
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #247   +/-   ##
=======================================
  Coverage   52.63%   52.63%           
=======================================
  Files           2        2           
  Lines          19       19           
=======================================
  Hits           10       10           
  Misses          9        9           

Copy link
Collaborator

@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.

These should be templated. Let's loop in Bob

@bobleesj
Copy link
Collaborator

bobleesj commented Jan 7, 2025

@sbillinge @cadenmyers13

yes lgtm. the modification is in cookiecutter.json so the new email (correct) will be all templated throughout once a new proj is created running scikit-package.

@sbillinge
Copy link
Collaborator

@sbillinge @cadenmyers13

yes lgtm. the modification is in cookiecutter.json so the new email (correct) will be all templated throughout once a new proj is created running scikit-package.

Ah, I see, these are the user prompts. I think we should maybe remove my name from these altogether perhaps, and use more generic exemplar names?

@sbillinge
Copy link
Collaborator

On the other hand, if we are the main users of this, having defaults that work for us would be good.

How about an infrastructure where we can have a config.json file where you can define local defaults?

@bobleesj
Copy link
Collaborator

bobleesj commented Jan 8, 2025

@sbillinge

I think we should maybe remove my name from these altogether perhaps

I think having specific examples can indeed help reduce cognitive overload and for the email, it provides a sense of who is currently maintaining and developing this software as a contact point.

How about an infrastructure where we can have a config.json file where you can define local defaults?

We could try to see if this approach works and whether we can import information into cookiecutter.json from another file. This will be actually semi-required for package update to be seamless.

@bobleesj
Copy link
Collaborator

bobleesj commented Jan 8, 2025

@sbillinge created issue #248. This can be addressed while also addressing #237 (package update)

If we are okay with the default values of sbillinge info, this PR could be merged.

@sbillinge sbillinge merged commit 5b15883 into Billingegroup:main Jan 8, 2025
5 checks passed
@sbillinge
Copy link
Collaborator

@bobleesj in light of your comments, maybe we can leave my name there in general. I agree that a specific example is better than a generic one. But I also like the idea of a local config option. I could imagnie the "Bob Lee Group" in the future and you would much rather Bob Lee is the default than Simon Billinge for you and all your students.......

@bobleesj
Copy link
Collaborator

bobleesj commented Jan 8, 2025

Yes agreed!

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.

3 participants