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

Put reused filenames in constants #194

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion butane/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,13 @@ pub use butane_core::{
Result, SqlType, SqlVal, SqlValRef, ToSql,
};

pub mod _filenames {
//! Filenames used in butane.
pub use butane_core::_filenames::*;
}

pub mod db {
//! Database helpers
//! Database helpers.
pub use butane_core::db::*;
}

Expand Down
5 changes: 3 additions & 2 deletions butane_cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::{
path::{Path, PathBuf},
};

use butane::_filenames::{BUTANE_DIRNAME, MIGRATIONS_DIRNAME};
use butane::migrations::{
copy_migration, FsMigrations, MemMigrations, Migration, MigrationMut, Migrations, MigrationsMut,
};
Expand Down Expand Up @@ -304,7 +305,7 @@ pub fn clean(base_dir: &Path) -> Result<()> {
}

pub fn get_migrations(base_dir: &Path) -> Result<FsMigrations> {
let root = base_dir.join("migrations");
let root = base_dir.join(MIGRATIONS_DIRNAME);
if !root.exists() {
eprintln!("No butane migrations directory found. Add at least one model to your project and build.");
std::process::exit(1);
Expand Down Expand Up @@ -342,7 +343,7 @@ pub fn find_butane_workspace_member_paths() -> Result<Vec<PathBuf>> {
// Find all workspace member with a .butane
for member in workspace_members {
let package_dir = extract_package_directory(&metadata.packages, member)?;
let member_butane_dir = package_dir.join(".butane/");
let member_butane_dir = package_dir.join(BUTANE_DIRNAME);

if member_butane_dir.exists() {
possible_directories.push(package_dir);
Expand Down
3 changes: 2 additions & 1 deletion butane_cli/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::path::PathBuf;

use butane::_filenames::BUTANE_DIRNAME;
use butane_cli::{
base_dir, clean, clear_data, collapse_migrations, delete_table, detach_latest_migration, embed,
get_migrations, handle_error, list_migrations, migrate, Result,
Expand Down Expand Up @@ -109,7 +110,7 @@ However if the migration has been manually edited, it will need to be manually r
.arg_required_else_help(true);
let args = app.get_matches();
let mut base_dir = args.get_one::<PathBuf>("path").unwrap().clone();
base_dir.push(".butane");
base_dir.push(BUTANE_DIRNAME);

// List any detached migrations.
if let Ok(ms) = get_migrations(&base_dir) {
Expand Down
5 changes: 4 additions & 1 deletion butane_cli/tests/embed.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use butane::_filenames::BUTANE_DIRNAME;

#[test]
fn working_dir_path() {
let path = butane_cli::working_dir_path();
Expand All @@ -8,7 +10,8 @@ fn working_dir_path() {
fn embed() {
let example_dir = std::env::current_dir()
.unwrap()
.join("../examples/getting_started/.butane");
.join("../examples/getting_started")
.join(BUTANE_DIRNAME);
assert!(example_dir.exists());
butane_cli::embed(&example_dir).unwrap();
}
5 changes: 3 additions & 2 deletions butane_codegen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ extern crate proc_macro;

use std::path::PathBuf;

use butane_core::_filenames::*;
use butane_core::migrations::adb::{DeferredSqlType, TypeIdentifier};
use butane_core::{codegen, make_compile_error, migrations, SqlType};
use proc_macro::TokenStream;
Expand Down Expand Up @@ -219,8 +220,8 @@ fn migrations_dir() -> PathBuf {
let mut dir = PathBuf::from(
std::env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR expected to be set"),
);
dir.push(".butane");
dir.push("migrations");
dir.push(BUTANE_DIRNAME);
dir.push(MIGRATIONS_DIRNAME);
dir
}

Expand Down
6 changes: 4 additions & 2 deletions butane_core/src/db/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use std::path::Path;

use serde::{Deserialize, Serialize};

use crate::_filenames::CONNECTION_JSON_FILENAME;
use crate::query::BoolExpr;
use crate::{migrations::adb, Error, Result, SqlVal, SqlValRef};

Expand Down Expand Up @@ -112,7 +113,7 @@ impl ConnectionSpec {
f.write_all(serde_json::to_string_pretty(self)?.as_bytes())
.map_err(|e| e.into())
}
/// Load a previously saved connection spec
/// Load a previously saved connection spec.
pub fn load(path: impl AsRef<Path>) -> Result<Self> {
let path = conn_complete_if_dir(path.as_ref());
serde_json::from_reader(fs::File::open(path)?).map_err(|e| e.into())
Expand All @@ -127,7 +128,8 @@ impl ConnectionSpec {

fn conn_complete_if_dir(path: &Path) -> Cow<Path> {
if path.is_dir() {
Cow::from(path.join("connection.json"))
//let path = path.join(CONNECTION_JSON_FILENAME);
Cow::from(path.join(CONNECTION_JSON_FILENAME))
} else {
Cow::from(path)
}
Expand Down
9 changes: 9 additions & 0 deletions butane_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ use db::{BackendRow, Column, ConnectionMethods};
pub use query::Query;
pub use sqlval::{AsPrimaryKey, FieldType, FromSql, PrimaryKeyType, SqlVal, SqlValRef, ToSql};

/// Filenames used in butane.
pub mod _filenames {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Using _ to indicate semi-private nature

/// Directory to hold butane metadata.
pub const BUTANE_DIRNAME: &'static str = ".butane";
/// Filename for storing connection metadata.
pub const CONNECTION_JSON_FILENAME: &'static str = "connection.json";
/// Directory to hold butane migration data.
pub const MIGRATIONS_DIRNAME: &'static str = "migrations";
}
/// Result type that uses [`crate::Error`].
pub type Result<T> = std::result::Result<T, crate::Error>;

Expand Down
4 changes: 3 additions & 1 deletion butane_test_helper/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::path::PathBuf;
use std::process::{ChildStderr, Command, Stdio};
use std::sync::Mutex;

use butane_core::_filenames::{BUTANE_DIRNAME, MIGRATIONS_DIRNAME};
use butane_core::db::{connect, get_backend, pg, sqlite, Backend, Connection, ConnectionSpec};
use butane_core::migrations::{self, MemMigrations, Migration, Migrations, MigrationsMut};
use once_cell::sync::Lazy;
Expand Down Expand Up @@ -159,7 +160,8 @@ pub fn pg_connstr(data: &PgSetupData) -> String {

pub fn setup_db(backend: Box<dyn Backend>, conn: &mut Connection, migrate: bool) {
let mut root = std::env::current_dir().unwrap();
root.push(".butane/migrations");
root.push(BUTANE_DIRNAME);
root.push(MIGRATIONS_DIRNAME);
let mut disk_migrations = migrations::from_root(&root);
let disk_current = disk_migrations.current();
eprintln!("{:?}", disk_current);
Expand Down
1 change: 1 addition & 0 deletions docs/getting-started.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ connection parameters. At this point, we can add a method (in our
use butane::db::{Connection, ConnectionSpec};

pub fn establish_connection() -> Connection {
let connection_spec = format!(".butane/{CONNECTIONS_JSON_FILENAME}");
butane::db::connect(&ConnectionSpec::load(".butane/connection.json").unwrap()).unwrap()
}
```
Expand Down
4 changes: 3 additions & 1 deletion example/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#![allow(dead_code)]

use butane::_filenames::BUTANE_DIRNAME;
use butane::db::{Connection, ConnectionSpec};
use butane::prelude::*;
use butane::{find, model, query, AutoPk, Error, ForeignKey, Many};
Expand Down Expand Up @@ -86,7 +88,7 @@ fn query() -> Result<()> {

fn establish_connection() -> Result<Connection> {
let mut cwd = std::env::current_dir()?;
cwd.push(".butane");
cwd.push(BUTANE_DIRNAME);
let spec = ConnectionSpec::load(cwd)?;
let conn = butane::db::connect(&spec)?;
Ok(conn)
Expand Down
3 changes: 2 additions & 1 deletion example/tests/test_cli.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use assert_cmd::Command;
use butane::_filenames::{BUTANE_DIRNAME, CONNECTION_JSON_FILENAME};

#[test]
fn test_migrate_and_query() {
let db = "db.sqlite";
let connspec = ".butane/connection.json";
let connspec = format!("{BUTANE_DIRNAME}/{CONNECTION_JSON_FILENAME}");

// These files should have been removed by build.rs
assert!(!std::path::Path::new(&db).exists());
Expand Down
4 changes: 3 additions & 1 deletion examples/getting_started/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
pub mod butane_migrations;
pub mod models;

use butane::_filenames::{BUTANE_DIRNAME, CONNECTION_JSON_FILENAME};
use butane::db::{Connection, ConnectionSpec};
use butane::prelude::*;
use models::{Blog, Post};

/// Load a [Connection].
pub fn establish_connection() -> Connection {
butane::db::connect(&ConnectionSpec::load(".butane/connection.json").unwrap()).unwrap()
let connspec = format!("{BUTANE_DIRNAME}/{CONNECTION_JSON_FILENAME}");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IMO the fact that the examples need to include these strings feels like a problem.
These shouldnt need to be so visible - they should be a hidden detail that are only important for unusual usage.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think ConnectionSpec::load should auto detect if this filename exists

butane::db::connect(&ConnectionSpec::load(connspec).unwrap()).unwrap()
}

/// Create a [Blog].
Expand Down
Loading