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

Use changer::changer to rename package #22

Merged
merged 2 commits into from
Nov 27, 2023
Merged

Use changer::changer to rename package #22

merged 2 commits into from
Nov 27, 2023

Conversation

PietrH
Copy link
Member

@PietrH PietrH commented Nov 27, 2023

Me and @SanderDevisscher have decided to rename the repo, and package from vespawatchr to iassetR because while we currently are writing this in the scope of vespawatch, we want to keep the package general enough to be used by any iAsset user. Any vespawatch specific code should go in vespawatch specific repo's.

@SanderDevisscher , I'm setting you as reviewer as soon as this is ready for review. But I would like to merge it into main myself.

From this merge onwards, anyone developing on this package will have to update their local versions to the new repo and project. You can do this in Git CLI or Github Desktop (under repository), but I usually find it easier to just delete my local copy and to clone a new one.

@PietrH PietrH linked an issue Nov 27, 2023 that may be closed by this pull request
@PietrH PietrH self-assigned this Nov 27, 2023
@PietrH
Copy link
Member Author

PietrH commented Nov 27, 2023

Sander, here is what I think you need to test:

  • Does this break you local version of the package (yes!): can you get it to work again by cloning
  • Does this branch pass R CMD CHECK for you, this is under the build tab of RStudio
  • Can you get any function to break that wasn't already broken after this change
  • Does this break the other (hopefully very experimental) scripts that you have that are dependent on iassetR
  • Did I miss anything while renaming the package?

Also:

How are we going to handle references to vespawatch going forward, ie in the function documentation and examples. In default values etc. I say we keep them in for now, but it might be a pain removing them later. It's a balance of user friendliness towards ourselves as users, and possible other users that don't know about vespawatch in the future.

We might want to avoid setting default values to vespawatch defaults, but it would be handy for us... We could also handle this with System ENV parameters, but I always find that less user friendly when I'm using a new package since this is stuff I can't really see in my IDE. What do you think?

@PietrH PietrH marked this pull request as ready for review November 27, 2023 10:30
@SanderDevisscher
Copy link
Collaborator

@PietrH I can plan to test this by this wednesday (29/11) ok?

@PietrH
Copy link
Member Author

PietrH commented Nov 27, 2023

Alright, It's better not to merge anything to main in the meantime. I want any merge conflicts to occur with these changes already present in the main branch.

Copy link
Collaborator

@SanderDevisscher SanderDevisscher left a comment

Choose a reason for hiding this comment

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

─ R CMD check results iassetR 0.0.0.─
Duration: 17.7s

0 errors ✔ | 0 warnings ✔ | 0 notes ✔

R CMD check succeeded

After cloning & building the package from this branch just changing library(vespawatchr) to library(iassetR) in my experimental scripts seems to work like a charm.

Are there any more tests I need to do before merging ?

@PietrH
Copy link
Member Author

PietrH commented Nov 27, 2023

Nope, all good. I'll merge this one myself!

@PietrH PietrH merged commit f0a3b2e into main Nov 27, 2023
6 checks passed
@PietrH PietrH deleted the rename-package branch November 27, 2023 16:39
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.

Come up with a better name for this package
2 participants